Merge pull request 'fix: db volume ownership and explicit error handling for write failures' (#3) from fix/db-permissions-and-error-handling into dev

Reviewed-on: #3

Dockerfile

@@ -10,7 +10,7 @@ COPY . .
 RUN awk -F'"' '/"version"/{printf "const VERSION = \"%s\";\n", $4; exit}' \
     package.json > js/version.js
 
-RUN chown -R app:app /app
+RUN mkdir -p /app/data && chown -R app:app /app
 USER app
 
 EXPOSE 3000

server/routes.js

@@ -78,7 +78,8 @@ router.post('/instances', (req, res) => {
   } catch (e) {
     if (e.message.includes('UNIQUE')) return res.status(409).json({ error: 'vmid already exists' });
     if (e.message.includes('CHECK'))  return res.status(400).json({ error: 'invalid field value' });
-    throw e;
+    console.error('POST /api/instances', e);
+    res.status(500).json({ error: 'internal server error' });
   }
 });
 
@@ -98,7 +99,8 @@ router.put('/instances/:vmid', (req, res) => {
   } catch (e) {
     if (e.message.includes('UNIQUE')) return res.status(409).json({ error: 'vmid already exists' });
     if (e.message.includes('CHECK'))  return res.status(400).json({ error: 'invalid field value' });
-    throw e;
+    console.error('PUT /api/instances/:vmid', e);
+    res.status(500).json({ error: 'internal server error' });
   }
 });
 
@@ -112,6 +114,11 @@ router.delete('/instances/:vmid', (req, res) => {
   if (instance.stack !== 'development')
     return res.status(422).json({ error: 'only development instances can be deleted' });
 
+  try {
     deleteInstance(vmid);
     res.status(204).end();
+  } catch (e) {
+    console.error('DELETE /api/instances/:vmid', e);
+    res.status(500).json({ error: 'internal server error' });
+  }
 });

tests/api.test.js

@@ -1,7 +1,8 @@
-import { describe, it, expect, beforeEach } from 'vitest'
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'
 import request from 'supertest'
 import { app } from '../server/server.js'
 import { _resetForTest } from '../server/db.js'
+import * as dbModule from '../server/db.js'
 
 beforeEach(() => _resetForTest())
 
@@ -277,3 +278,74 @@ describe('static assets and SPA routing', () => {
     expect(res.text).toContain('<base href="/">')
   })
 })
+
+// ── Error handling — unexpected DB failures ───────────────────────────────────
+
+const dbError = () => Object.assign(
+  new Error('attempt to write a readonly database'),
+  { code: 'ERR_SQLITE_ERROR', errcode: 8 }
+)
+
+describe('error handling — unexpected DB failures', () => {
+  let consoleSpy
+
+  beforeEach(() => {
+    consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
+  })
+
+  afterEach(() => {
+    vi.restoreAllMocks()
+  })
+
+  it('POST returns 500 with friendly message when DB throws unexpectedly', async () => {
+    vi.spyOn(dbModule, 'createInstance').mockImplementationOnce(() => { throw dbError() })
+    const res = await request(app).post('/api/instances').send(base)
+    expect(res.status).toBe(500)
+    expect(res.body).toEqual({ error: 'internal server error' })
+  })
+
+  it('POST logs the error with route context when DB throws unexpectedly', async () => {
+    vi.spyOn(dbModule, 'createInstance').mockImplementationOnce(() => { throw dbError() })
+    await request(app).post('/api/instances').send(base)
+    expect(consoleSpy).toHaveBeenCalledWith(
+      expect.stringContaining('POST /api/instances'),
+      expect.any(Error)
+    )
+  })
+
+  it('PUT returns 500 with friendly message when DB throws unexpectedly', async () => {
+    await request(app).post('/api/instances').send(base)
+    vi.spyOn(dbModule, 'updateInstance').mockImplementationOnce(() => { throw dbError() })
+    const res = await request(app).put('/api/instances/100').send(base)
+    expect(res.status).toBe(500)
+    expect(res.body).toEqual({ error: 'internal server error' })
+  })
+
+  it('PUT logs the error with route context when DB throws unexpectedly', async () => {
+    await request(app).post('/api/instances').send(base)
+    vi.spyOn(dbModule, 'updateInstance').mockImplementationOnce(() => { throw dbError() })
+    await request(app).put('/api/instances/100').send(base)
+    expect(consoleSpy).toHaveBeenCalledWith(
+      expect.stringContaining('PUT /api/instances/:vmid'),
+      expect.any(Error)
+    )
+  })
+
+  it('DELETE returns 500 with friendly message when DB throws unexpectedly', async () => {
+    await request(app).post('/api/instances').send({ ...base, stack: 'development', state: 'testing' })
+    vi.spyOn(dbModule, 'deleteInstance').mockImplementationOnce(() => { throw dbError() })
+    const res = await request(app).delete('/api/instances/100')
+    expect(res.status).toBe(500)
+    expect(res.body).toEqual({ error: 'internal server error' })
+  })
+
+  it('DELETE logs the error with route context when DB throws unexpectedly', async () => {
+    await request(app).post('/api/instances').send({ ...base, stack: 'development', state: 'testing' })
+    vi.spyOn(dbModule, 'deleteInstance').mockImplementationOnce(() => { throw dbError() })
+    await request(app).delete('/api/instances/100')
+    expect(consoleSpy).toHaveBeenCalledWith(
+      expect.stringContaining('DELETE /api/instances/:vmid'),
+      expect.any(Error)
+    )
+  })
+})