Skip to content

feat(phase-8): implement Sentry integration and error tracking#9

Merged
Nether403 merged 1 commit into
mainfrom
feat/phase-8-batch-bcde
Jul 3, 2026
Merged

feat(phase-8): implement Sentry integration and error tracking#9
Nether403 merged 1 commit into
mainfrom
feat/phase-8-batch-bcde

Conversation

@Nether403

@Nether403 Nether403 commented Jul 3, 2026

Copy link
Copy Markdown
Owner

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.


Open in Devin Review

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

    • API: Init @sentry/node on boot; capture errors in onError with request-id; scrub idea and constraints; idempotent and no-op when SENTRY_DSN is unset.
    • Web: Init @sentry/react before React mounts; optional @sentry/vite-plugin for source maps gated by env; release comes from VITE_APP_RELEASE.
    • Auth: Production fails closed with 503 if Better Auth isn’t ready; session cookies use .stackfast.app in prod; admin/internal routes gate early.
    • Tests: Unit + property-based Sentry tests, app contract tests, E2E deploy specs, and a deploy smoke script.
  • Migration

    • API (Railway): set SENTRY_DSN (required to enable). Optional: SENTRY_RELEASE (else falls back to RAILWAY_GIT_COMMIT_SHA).
    • Web (build-time): set VITE_SENTRY_DSN and VITE_APP_RELEASE. Optional source-map upload: SENTRY_DSN, SENTRY_AUTH_TOKEN, SENTRY_ORG, SENTRY_PROJECT_WEB.
    • Redeploy both services. If any vars are missing, Sentry stays disabled and everything runs as before.

Written for commit 4b577e9. Summary will update on new commits.

Review in cubic

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.
@cubic-dev-ai

cubic-dev-ai Bot commented Jul 3, 2026

Copy link
Copy Markdown

This PR is large and would use a significant portion of your monthly review quota. Comment @cubic-dev-ai review this to confirm that you want cubic to review it.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 3 potential issues.

Open in Devin Review

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 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)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread apps/web/package.json
"@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",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread scripts/deploy/migrate.ts
Comment on lines +237 to +245
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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 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.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@Nether403 Nether403 merged commit f89716b into main Jul 3, 2026
2 checks passed
@Nether403 Nether403 deleted the feat/phase-8-batch-bcde branch July 3, 2026 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant