fix: db volume ownership and explicit error handling for write failures #3

Merged
josh merged 4 commits from fix/db-permissions-and-error-handling into dev 2026-03-28 12:10:32 -04:00
Owner

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

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>
josh added 1 commit 2026-03-28 11:12:10 -04:00
fix: db volume ownership and explicit error handling for write failures
All checks were successful
CI / test (pull_request) Successful in 9m32s
15ed329743
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>
josh added 1 commit 2026-03-28 11:16:32 -04:00
Merge branch 'dev' into fix/db-permissions-and-error-handling
Some checks failed
CI / test (pull_request) Failing after 4m52s
CI / build-dev (pull_request) Has been skipped
c6cd8098fd
josh scheduled this pull request to auto merge when all checks succeed 2026-03-28 11:16:58 -04:00
josh self-assigned this 2026-03-28 11:17:11 -04:00
josh added a new dependency 2026-03-28 11:33:43 -04:00
josh canceled auto merging this pull request when all checks succeed 2026-03-28 11:33:48 -04:00
josh added 1 commit 2026-03-28 11:48:20 -04:00
fix: skip db boot init in test env to prevent parallel worker lock
All checks were successful
CI / test (pull_request) Successful in 9m33s
CI / build-dev (pull_request) Has been skipped
08c12c9394
Vitest runs test files in parallel workers. Each worker imports server/db.js,
which triggered module-level init(DEFAULT_PATH) unconditionally. Two workers
racing to open the same SQLite file caused "database is locked", followed
by process.exit(1) killing the worker — surfacing as:

  Error: process.exit unexpectedly called with "1"

Fix: guard the boot init block behind NODE_ENV !== 'test'. Vitest sets
NODE_ENV=test automatically. Each worker's beforeEach(() => _resetForTest())
initialises its own :memory: database, so no file coordination is needed.

process.exit(1) is also guarded by the same condition — it must never
fire inside a test runner process.

TDD: two regression tests added to tests/db.test.js documenting the
expected boot behaviour and proving the module loads cleanly in parallel.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
josh added 1 commit 2026-03-28 11:48:39 -04:00
Merge branch 'dev' into fix/db-permissions-and-error-handling
All checks were successful
CI / test (pull_request) Successful in 9m28s
CI / build-dev (pull_request) Has been skipped
515ff8ddb3
josh scheduled this pull request to auto merge when all checks succeed 2026-03-28 11:48:49 -04:00
josh merged commit 73f4eabbc7 into dev 2026-03-28 12:10:32 -04:00
josh deleted branch fix/db-permissions-and-error-handling 2026-03-28 12:10:32 -04:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Reference: josh/Catalyst#3