fix(db): handle readonly database gracefully instead of crashing#235
Merged
fix(db): handle readonly database gracefully instead of crashing#235
Conversation
Contributor
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨Build
Other
Bug Fixes 🐛Telemetry
Other
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
Contributor
Codecov Results 📊✅ Patch coverage is 85.19%. Project has 4126 uncovered lines. Files with missing lines (71)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 68.74% 69.12% +0.38%
==========================================
Files 110 110 —
Lines 13124 13362 +238
Branches 0 0 —
==========================================
+ Hits 9021 9236 +215
- Misses 4103 4126 +23
- Partials 0 0 —Generated by Codecov Action |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Member
Author
|
@BYK lmk your thoughts on this one, i'd like to keep at least the notification to the user telling their sentry config db is readonly. |
BYK
reviewed
Feb 12, 2026
When the local database file is read-only (wrong permissions, read-only filesystem, etc.), DB writes now fail silently instead of crashing the CLI. A one-time warning is printed to stderr pointing users to `sentry cli fix`. The fix is centralized in the traced DB proxy (createTracedStatement), so all ~14 DB write paths are protected without touching individual modules. Reads continue working normally. Resolves CLI-4E: SQLiteError: attempt to write a readonly database
Extends `sentry cli fix` to detect and repair file permission issues on the local database. Checks the config directory (expects 0700), DB file, and WAL/SHM journals (expect 0600). Attempts chmod repair automatically; shows manual fix commands if that fails. This is the recommended fix path shown in the readonly DB warning.
…gDir - Use getConfigDir() instead of dirname(dbPath) for consistency with the canonical config path source - Remove node:path dirname import (no longer needed) - Add @param/@returns JSDoc to fix.ts helpers - Tighten warnReadonlyDatabaseOnce JSDoc (document no-op behavior) - Use explicit return undefined in proxy readonly handler - Trim obvious 'what' comments from test setup/teardown
Add 5 tests covering file/directory permission checking and repair: - Detect readonly database file permissions - Repair database file permissions (chmod 0444 -> 0600) - Detect directory permission issues - Dry-run reports issues without repairing - Handle combined permission and schema issues Also extract runFix() helper to reduce test boilerplate. Coverage for src/commands/cli/fix.ts: 100% functions, 88% lines.
The readonly error handler used a bare `return` for all traced methods, yielding `undefined` even for all() and values() which callers expect to return arrays. This would crash on .map()/.length if a write query ever used these methods (e.g. DELETE ... RETURNING *). Extract handleReadonlyError() helper that returns [] for all/values and undefined for run/get. Also reduces complexity in the proxy handler.
BYK
force-pushed
the
fix/readonly-db-handling
branch
from
64f244b to
300ceb8
Compare
- fix.ts: convert to async (stat/chmod from node:fs/promises), parallel checks with Promise.all/Promise.allSettled, inline fixFunc into buildCommand, explicit ENOENT handling - telemetry.ts: use let+noop self-replacing pattern for one-shot warning, add tryRepairReadonly() that chmods files for future commands before falling back to user warning - fix.test.ts: await async func.call() invocations - telemetry.test.ts: update readonly tests for auto-repair behavior
- Use exact mode match instead of bitmask in checkMode (prevents extra permission bits from passing validation) - Wrap handleSchemaIssues in try/catch so --dry-run doesn't crash on readonly databases - Repair config directory permissions in tryRepairReadonly alongside database files - Extract chmodIfExists helper that only catches ENOENT, re-throws other errors instead of swallowing them - Update tests: chmod db files to 0o600 after creation to match production setDbPermissions behavior
…ssues Previously, when handleSchemaIssues threw and no permission issues were detected, totalFound was 0 and the command reported 'No issues found' despite the schema check failure. Now anyFailed is checked alongside totalFound to ensure failures are always surfaced with a non-zero exit.
When the config directory lacks execute permission, stat on child files (db, wal, shm) fails with EACCES instead of returning mode bits. Treat EACCES like ENOENT (skip the file) since the directory permission check will catch the root cause.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Directory chmod must complete before child file chmod calls, otherwise file repairs can fail with EACCES when the parent directory lacks execute permission. Split repairPermissions into directory-first (sequential) then files (parallel) to prevent the race.
Add fix command tests: - Schema check failure with no permission issues sets exitCode=1 - Dry-run with schema failure sets exitCode=1 - Schema check suppresses error when permission issues explain failure - Schema repair success path (missing columns → repaired) Add telemetry test: - Readonly warning fallback when auto-repair fails (mock chmodSync to simulate unowned files)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When the CLI's local database (
~/.sentry/cli.db) is on a read-only filesystem or has wrong permissions, DB writes throwSQLiteError: attempt to write a readonly databaseand crash commands. Now the CLI catches these errors centrally in the traced DB proxy, warns the user once, and continues working — reads still succeed, only caching/persistence is lost.Closes https://sentry.sentry.io/issues/7258516238/
Changes
Readonly errors are caught in the traced DB proxy (
createTracedStatement) alongside the existing schema auto-repair logic. A one-time warning prints to stderr pointing users tosentry cli fix. This single change protects all ~14 DB write paths without touching individual modules.sentry cli fixnow also checks file permissions on the DB file, WAL/SHM journals, and config directory — and repairs them withchmodwhen possible.Test Plan
bun run typecheck— cleanbun run lint— cleanisReadonlyError()detection (5 cases), readonly proxy behavior (4 cases: no throw on write, reads work, warns once, message content)