Report ingestion-worker failures to Sentry with the app's privacy scrubbers - #1551
Conversation
…ubbers The worker is a separate Railway service that had no Sentry code at all, so OCR, embedding, and job-queue failures in production were visible only in Railway logs. It is a plain Node process rather than a Next runtime, so it initializes @sentry/node directly in worker/observability.ts, called from worker/index.ts before main.ts loads so module-level failures report too. Three failure paths capture, each tagged with a fixed stage label: - process: a single ingestion job threw (existing retry/fail handling unchanged — this only adds visibility) - claim: job claiming failed and the WORKER_MAX_CLAIM_FAILURES retry budget is spent; a single transient claim failure self-heals and is not reported - fatal: the worker loop stopped, flushed before exit alongside the existing WORKER_FAILURE_WEBHOOK_URL dispatch Privacy is stricter here than for the app because ingestion errors routinely quote storage paths, filenames, and extracted clinical text. Events reuse privacySafeErrorEvent, so the message is discarded; the only added tags are service=worker and worker_stage, both set from code literals and never from job, document, or owner data. SAFE_TAGS gains exactly those two keys, and worker events group by service + stage + runtime type + top frame since they have no route pattern. Tests pin that a document id, owner id, storage path, filename, and extracted text are all stripped. Also declares @sentry/node as a direct production dependency (the worker bundle keeps npm packages external, so it must resolve from prod node_modules) and corrects a stale docs claim that no source-map upload is configured — next.config.ts wires it, but it stays inert without SENTRY_AUTH_TOKEN/SENTRY_ORG/SENTRY_PROJECT, none of which are set today. Ledger #183 records the npm 11.6.2 lockfile-pruning trap that reddened every CI job on PR #1544, and the npm ci --dry-run workaround used here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SBBj97cbs3rNGk8BX3g1CV
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 46 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds Sentry tracking for worker bootstrap, claim, processing, and fatal failures. It applies privacy-safe tags and fingerprints, flushes events before exit, adds tests, updates documentation, and records repository ledger changes. ChangesWorker Sentry observability
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant WorkerBootstrap
participant WorkerMain
participant WorkerObservability
participant Sentry
WorkerBootstrap->>WorkerObservability: initialize before main import
WorkerMain->>WorkerObservability: capture worker exception
WorkerObservability->>Sentry: send scrubbed event
WorkerMain->>WorkerObservability: flush before exit
WorkerObservability->>Sentry: flush buffered events
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
🔒 Upgrade RequiredPrivate repository analysis requires Pro or Enterprise. Upgrade: https://ecc.tools/pricing?plan=pro ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers. |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SBBj97cbs3rNGk8BX3g1CV
🔒 Upgrade RequiredPrivate repository analysis requires Pro or Enterprise. Upgrade: https://ecc.tools/pricing?plan=pro ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers. |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #7708 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@cursoragent Unblock the current open PR. Confirm the PR number and GitHub head first from context. If more than one open PR could apply, stop and say which one you would use and why. Fetch and start from the remote tip that matches that GitHub head. If the named branch ref is missing or stale, use the PR head ref. Preserve unrelated local WIP; do not discard dirty work; do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge. No provider-backed gates without approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put blocker dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker and avoid cancelling in-flight required CI unless the push clears that blocker. If the PR is already merged or closed: confirm outcome, post one summary, and stop. Primary job: clear mergeability and required-CI blockers on this tip. Do a full product review only if needed to explain or fix a blocker. Stay light otherwise—no broad verification suites or Playwright unless needed to prove one concrete unblock fix. Snapshot the GitHub head: tip, behind/ahead, mergeable state, merge-tree versus origin/main (real conflict versus behind-but-clean), required checks on that tip including Production UI when selected, and unresolved threads that block merge or required CI. Missing checks while dirty are not green. Tip moved → re-snapshot. Ledger-lookup against that GitHub head under an unblock/fix scope for this PR. If already handled at this head for unblock work, merge-tree clean, required checks green, and no blocking threads: summarize, comment, stop. Unblock once: real conflict → merge origin/main with the smallest correct resolution (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. skip-branch-sync and similar labels do not block a manual sync needed to clear a real blocker. No sync thrash. After any sync or push, re-snapshot tip, merge-tree, and required checks before declaring done. Dedupe the ledger if a merge touched it. Fix only what blocks required checks or mergeability on this tip. Ignore advisory red unless it reveals a real required failure. Prefer the smallest safe fix; if a bot/autofix commit caused the break, prefer revert. Prove with the smallest targeted check only. Format and commit before push. Push only blocker-fix commits. Append ledger for the final GitHub head under the unblock scope. Never push a ledger-only tip. One PR comment: tip, sync/merge-tree, what blocked and what you fixed or dispositioned, required CI status, residual risks; merge left to me. Stop when merge-tree is clean on the current GitHub tip, no blocking threads remain for merge/required CI, and required checks are green or clearly in progress. |
|
Taking a look!   |
PR Summary by QodoReport ingestion-worker failures to Sentry using existing privacy scrubbers
AI Description
Diagram
High-Level Assessment
Files changed (11)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Co-authored-by: Cursor <cursoragent@cursor.com>
🔒 Upgrade RequiredPrivate repository analysis requires Pro or Enterprise. Upgrade: https://ecc.tools/pricing?plan=pro ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers. |
🔒 Upgrade RequiredPrivate repository analysis requires Pro or Enterprise. Upgrade: https://ecc.tools/pricing?plan=pro ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers. |
|
@codex resolve merge conflicts and fix any CI failing here and merge safely to main |
|
Codex couldn't complete this request. Try again later. |
Resolve docs/error-tracking.md by keeping both the ingestion-worker section and main's Agent Monitoring wizard mapping. Resolve docs/outstanding-issues.md by keeping main's open rows, removing the blank line that orphaned #183/#186 outside the open table, renumbering this PR's npm lockfile issue to #204, and bumping issues:next-id to 205. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
🔒 Upgrade RequiredPrivate repository analysis requires Pro or Enterprise. Upgrade: https://ecc.tools/pricing?plan=pro ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/worker-observability.test.ts (1)
10-20: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd coverage for the guarded path when Sentry is configured but the SDK call itself throws.
This test only covers the branch where
SENTRY_DSNis unset, soinitWorkerErrorTracking()short-circuits and the underlying@sentry/nodecalls never execute. The stack description states these tests should cover "guarded operations," which implies verifying thatcaptureWorkerExceptionandflushWorkerErrorTrackingalso swallow errors when Sentry is initialized and the SDK itself throws (for example, a network failure during flush). Add a second test that stubsSENTRY_DSNto a non-empty value, mocks@sentry/nodeto throw fromcaptureException/flush, and asserts the wrapper functions still do not throw.🧪 Suggested additional test
it("does not throw when Sentry itself throws during capture or flush", async () => { vi.stubEnv("SENTRY_DSN", "https://example@ingest.example.com/1"); vi.doMock("`@sentry/node`", () => ({ init: vi.fn(), captureException: vi.fn(() => { throw new Error("sdk failure"); }), flush: vi.fn(() => Promise.reject(new Error("flush failure"))), })); const { initWorkerErrorTracking, captureWorkerException, flushWorkerErrorTracking } = await import("../worker/observability"); expect(initWorkerErrorTracking()).toBe(true); expect(() => captureWorkerException(new Error("boom"), "process")).not.toThrow(); await expect(flushWorkerErrorTracking()).resolves.toBeUndefined(); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/worker-observability.test.ts` around lines 10 - 20, Add a second test alongside the existing inert-path test that sets a non-empty SENTRY_DSN, mocks `@sentry/node` so captureException throws and flush rejects, then imports the observability wrappers and verifies initWorkerErrorTracking returns true while captureWorkerException and flushWorkerErrorTracking do not throw.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/outstanding-issues.md`:
- Line 225: Update the recommended execution queue in docs/outstanding-issues.md
to include ledger row `#204`, preserving the queue’s ordering and formatting
conventions. If the queue is intended to contain every open ledger row,
reconcile it so `#204` is represented before merging.
---
Nitpick comments:
In `@tests/worker-observability.test.ts`:
- Around line 10-20: Add a second test alongside the existing inert-path test
that sets a non-empty SENTRY_DSN, mocks `@sentry/node` so captureException throws
and flush rejects, then imports the observability wrappers and verifies
initWorkerErrorTracking returns true while captureWorkerException and
flushWorkerErrorTracking do not throw.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1db50bb6-c7d3-4757-8d4f-62f581e116cd
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
docs/branch-review-ledger.mddocs/codebase-index.mddocs/error-tracking.mddocs/outstanding-issues.mdpackage.jsonsrc/lib/observability/error-tracking.tstests/worker-observability.test.tsworker/index.tsworker/main.tsworker/observability.ts
CodeRabbit correctly noted the queue claims to list every open ledger row but omitted the npm lockfile/CI issue added during the main merge. Co-authored-by: Cursor <cursoragent@cursor.com>
🔒 Upgrade RequiredPrivate repository analysis requires Pro or Enterprise. Upgrade: https://ecc.tools/pricing?plan=pro ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers. |
2 participants
Summary
.dark .ckb-v2after the cascade port so light.ckb-v2matching inside dark subtrees cannot resolve light ink on dark surfaces.origin/mainto clear DIRTY mergeability (prefer main’s post-feat(design-system): v2 token layer + 26 components — reconciled, cascade-ported, forced-colours owned #1538/fix(design-system): critique fixes + post-merge truth sweep (bundled hygiene PR) #1541 component copies; keep this tip’s dark-ink contract + HCM transparent overlay-backdrop rationale).aria-controlsonly on the selected tab when a panel is owned (unselected tabs no longer point at missing IDs).RAG impact: no retrieval behaviour change — design-system token dark-cascade fix and unadopted UI component a11y only; no file under
src/lib/rag/**, clinical-search, retrieval-selection, ranking, eval harness, golden fixtures, or retrieval RPCs is touched.Verification
npm run verify:pr-localvitesttests/ckb-v2-token-contract.test.ts+tests/ui-v2-components.dom.test.tsx— 35 passednpm run verify:cheap— 457 files / 4782 passed.ckb-v2/ these components yet; phone-chrome and visual journeys unchanged. Prefer CI Production UI on this tip.Risk and rollout
Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
Summary by CodeRabbit
New Features
Documentation