feat(phase-8): implement Sentry integration and error tracking#9
Conversation
This commit introduces Sentry for error tracking in the API and web applications, enhancing observability. Key changes include: - Added Sentry initialization in both the API and web entry points, ensuring it captures errors before any requests are processed. - Configured Sentry to operate conditionally based on the presence of environment variables, allowing for a no-op state when not in use. - Updated the error handling in the API to capture exceptions and correlate them with request IDs for better traceability. - Enhanced the authentication middleware to fail closed in production if the auth system is misconfigured, returning a 503 status. Additionally, the README and environment variable documentation have been updated to reflect the new Sentry configuration requirements. Closes tasks B1, B2, B3, and B4 from the Phase 8 deployment spec.
|
This PR is large and would use a significant portion of your monthly review quota. Comment |
There was a problem hiding this comment.
🚩 Asymmetric error handling: only requireSession wraps getAuth() in try-catch
The PR adds a try-catch around getAuth() in requireSession() (apps/api/src/middleware/auth.ts:150-157) so that a throw from Better Auth construction is treated as auth = null and fails closed in production. However, two other call sites — optionalSession() at apps/api/src/middleware/auth.ts:228 and the /api/auth/* route handler at apps/api/src/app.ts:144 — still call getAuth() without a try-catch. If createAuth() throws in those contexts, the error propagates to the onError handler and returns a 500 instead of a more graceful response. This is pre-existing behavior that the PR chose not to change, but the asymmetry could surprise future maintainers.
(Refers to lines 227-228)
Was this helpful? React with 👍 or 👎 to provide feedback.
| "@radix-ui/react-toast": "^1.2.15", | ||
| "@radix-ui/react-tooltip": "^1.2.8", | ||
| "@sentry/react": "^10.53.1", | ||
| "@sentry/vite-plugin": "^4.0.0", |
There was a problem hiding this comment.
🟡 Build-time Vite plugin incorrectly listed as a runtime dependency, bloating the production install
The source-map upload plugin is listed as a runtime dependency (@sentry/vite-plugin at apps/web/package.json:21) instead of a dev dependency, so it and its transitive packages are installed in production even though the plugin is only used during vite build.
Impact: Unnecessary install bloat on the production build image; inconsistent with the existing convention where other Vite plugins (@vitejs/plugin-react, vite) are in devDependencies.
Convention mismatch with existing Vite plugin placement
The other Vite build-time plugin @vitejs/plugin-react is correctly placed in devDependencies at apps/web/package.json:42. The @sentry/vite-plugin is only imported in apps/web/vite.config.ts:4 and used conditionally in the sentryPlugins() helper at apps/web/vite.config.ts:24. It is never imported by any runtime source file under apps/web/src/.
Prompt for agents
Move @sentry/vite-plugin from dependencies to devDependencies in apps/web/package.json. It is a build-time-only Vite plugin used only in vite.config.ts, not a runtime dependency. This matches the existing convention where @vitejs/plugin-react and vite itself are in devDependencies.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const armIdle = (): void => { | ||
| if (idleTimer) clearTimeout(idleTimer); | ||
| idleTimer = setTimeout(() => { | ||
| // drizzle-kit has printed the diff and is now blocked waiting for | ||
| // approval. We never approve — abort so nothing is applied. | ||
| abortedAtPrompt = true; | ||
| log("dry-run: reached approval prompt; aborting without applying"); | ||
| killTree(child); | ||
| }, DRY_RUN_IDLE_MS); |
There was a problem hiding this comment.
🚩 Migration runner's dry-run relies on idle-timeout heuristic to detect the approval prompt
The runDryRun function at scripts/deploy/migrate.ts:229-295 uses a 3-second idle timer (DRY_RUN_IDLE_MS) to detect when drizzle-kit push --strict has stopped producing output and is waiting at the approval prompt. If drizzle-kit takes longer than 3 seconds between output chunks (e.g. due to a slow schema diff on a large database), the script would prematurely kill the process and report success even though the DDL wasn't fully printed. The 60-second hard timeout (DRY_RUN_HARD_MS) provides a safety net, but the idle heuristic could produce incomplete dry-run output under load.
Was this helpful? React with 👍 or 👎 to provide feedback.
This commit introduces Sentry for error tracking in the API and web applications, enhancing observability. Key changes include:
Additionally, the README and environment variable documentation have been updated to reflect the new Sentry configuration requirements.
Closes tasks B1, B2, B3, and B4 from the Phase 8 deployment spec.
Summary by cubic
Adds Sentry to the API and web with safe, no‑op defaults and request‑id tagging to improve error visibility. Also hardens auth to fail closed in production when misconfigured.
New Features
@sentry/nodeon boot; capture errors inonErrorwith request-id; scrubideaandconstraints; idempotent and no-op whenSENTRY_DSNis unset.@sentry/reactbefore React mounts; optional@sentry/vite-pluginfor source maps gated by env; release comes fromVITE_APP_RELEASE..stackfast.appin prod; admin/internal routes gate early.Migration
SENTRY_DSN(required to enable). Optional:SENTRY_RELEASE(else falls back toRAILWAY_GIT_COMMIT_SHA).VITE_SENTRY_DSNandVITE_APP_RELEASE. Optional source-map upload:SENTRY_DSN,SENTRY_AUTH_TOKEN,SENTRY_ORG,SENTRY_PROJECT_WEB.Written for commit 4b577e9. Summary will update on new commits.