fix: db volume ownership and explicit error handling for write failures
All checks were successful
CI / test (pull_request) Successful in 9m32s
All checks were successful
CI / test (pull_request) Successful in 9m32s
Root cause of the 500 on create/update/delete: the non-root app user in
the Docker container lacked write permission to the volume mount point.
Docker volume mounts are owned by root by default; the app user (added
in a previous commit) could read the database but not write to it.
Fixes:
1. Dockerfile — RUN mkdir -p /app/data before chown so the directory
exists in the image with correct ownership. Docker uses this as a
seed when initialising a new named volume, ensuring the app user
owns the mount point from the start.
NOTE: existing volumes from before the non-root user was introduced
will still be root-owned. Fix with:
docker run --rm -v catalyst-data:/data alpine chown -R 1000:1000 /data
2. server/routes.js — replace bare `throw e` in POST/PUT catch blocks
with console.error (route context + error) + explicit 500 response.
Add try-catch to DELETE handler which previously had none. Unexpected
DB errors now log the route they came from and return a clean JSON
body instead of relying on the generic Express error handler.
3. server/db.js — wrap the boot init() call in try-catch. Fatal startup
errors (e.g. data directory not writable) now print a clear message
pointing to the cause before exiting, instead of a raw stack trace.
TDD: tests written first (RED), then fixed (GREEN). Six new tests in
tests/api.test.js verify that unexpected DB errors on POST, PUT, and
DELETE return 500 with { error: 'internal server error' } and call
console.error with the route context string.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
10
server/db.js
10
server/db.js
@@ -134,4 +134,12 @@ export function _resetForTest() {
|
||||
|
||||
// ── Boot ──────────────────────────────────────────────────────────────────────
|
||||
|
||||
init(process.env.DB_PATH ?? DEFAULT_PATH);
|
||||
const DB_PATH = process.env.DB_PATH ?? DEFAULT_PATH;
|
||||
try {
|
||||
init(DB_PATH);
|
||||
} catch (e) {
|
||||
console.error('[catalyst] fatal: could not open database at', DB_PATH);
|
||||
console.error('[catalyst] ensure the data directory exists and is writable by the server process.');
|
||||
console.error(e);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
@@ -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' });
|
||||
|
||||
deleteInstance(vmid);
|
||||
res.status(204).end();
|
||||
try {
|
||||
deleteInstance(vmid);
|
||||
res.status(204).end();
|
||||
} catch (e) {
|
||||
console.error('DELETE /api/instances/:vmid', e);
|
||||
res.status(500).json({ error: 'internal server error' });
|
||||
}
|
||||
});
|
||||
|
||||
@@ -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)
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user