feat(observability): fan-out errors Sentry + PostHog + opt-in flags#3489
feat(observability): fan-out errors Sentry + PostHog + opt-in flags#3489PierreBrisorgueil merged 3 commits intomasterfrom
Conversation
…3486) - New `lib/services/errorTracker.js`: `captureException(err, ctx)` fans out to Sentry (if dsn) and PostHog (if apiKey + errorTracking===true). Silent no-op when neither is configured. `setupExpressErrorHandler` replaces the previous Sentry-only wiring in express.js. - `analytics.js`: add `captureException(err, ctx)` — sends PostHog `$exception` event with message, type, stack, requestId. - `express.js`: gate `analyticsMiddleware` behind `posthog.autoCapture===true` (was unconditionally mounted); replace `sentry.setupExpressErrorHandler` with `errorTracker.setupExpressErrorHandler` (Sentry handler + fan-out middleware). - Config defaults: add `posthog.errorTracking: false` and `posthog.autoCapture: false` explicit opt-in flags — default-safe invariant. - Tests: 11 unit tests covering all 4 tracker combinations + init + Express middleware, including default-safe invariant verification. Closes #3486
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 38 minutes and 25 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3489 +/- ##
==========================================
+ Coverage 85.85% 85.95% +0.10%
==========================================
Files 115 116 +1
Lines 2919 2948 +29
Branches 809 826 +17
==========================================
+ Hits 2506 2534 +28
- Misses 327 328 +1
Partials 86 86 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR adds an error-tracking “fan-out” layer so runtime exceptions can be reported to Sentry and/or PostHog, while making PostHog behaviors explicitly opt-in via config flags (default-safe when only posthog.apiKey is set).
Changes:
- Introduces
lib/services/errorTracker.jsto fan-outcaptureException()to Sentry and PostHog (gated by config) and to mount an Express error handler. - Adds
captureException()support tolib/services/analytics.js(PostHog$exceptionevents). - Makes PostHog auto-capture middleware opt-in via
posthog.autoCapture === true, and adds dev defaults forposthog.errorTracking/posthog.autoCapture. - Adds unit tests covering tracker combinations and the default-safe invariant.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/services/tests/errorTracker.unit.tests.js | Adds unit tests for errorTracker fan-out and Express error-handler wiring. |
| lib/services/express.js | Gates PostHog auto-capture middleware behind an opt-in flag; switches to errorTracker error handler. |
| lib/services/errorTracker.js | New service that fans out exception capture to Sentry and PostHog and mounts Express error middleware. |
| lib/services/analytics.js | Adds PostHog $exception capture support. |
| config/defaults/development.config.js | Adds explicit opt-in defaults for PostHog error tracking and auto-capture. |
Add 4 tests covering captureException: correct $exception event payload, anonymous fallback distinctId, no-op when client not initialised, and silent error swallowing. Ensures codecov/patch threshold is met.
…s rename, req.user._id - `errorTracker.js`: add `captureExceptionPostHogOnly` to avoid double-reporting to Sentry in the Express error middleware (Sentry already captured via its own handler). Rename unused `res` to `_res`. Derive `distinctId` from `req.user._id` / `req.user.id` (not the non-existent `req.userId`). - Tests: update setupExpressErrorHandler test to verify Sentry is NOT called from middleware. Add 2 new tests for req.user.id fallback and anonymous.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Overview
The pull request is currently in a state that prevents a meaningful review because the provided code diff is empty. No files were included in the submission, which makes it impossible to verify the implementation of the lib/services/errorTracker.js service, the updates to analytics.js, or the Express middleware gating logic.
While the intent describes a robust 'default-safe' observability architecture, none of the seven core acceptance criteria can be confirmed. Additionally, the 11 unit tests mentioned in the PR description are not present in the diff. This missing implementation is a critical blocker that must be resolved before the PR can proceed.
About this PR
- The provided code diff is empty. It is currently impossible to verify the implementation of the error tracking service, the logic for configuration-gated fan-outs to Sentry and PostHog, or the presence of the unit tests mentioned in the description. Please ensure the changes are correctly pushed and included in the PR.
Test suggestions
- Error is sent to Sentry when DSN is configured
- Error is sent to PostHog only when apiKey is present and errorTracking flag is true
- Error tracking results in a silent no-op when no configurations are provided
- PostHog analytics middleware is bypassed when autoCapture flag is false
- CaptureException in analytics.js correctly formats message, type, stack, and requestId for PostHog
- Verify 'default-safe' invariant: flags default to false when only the apiKey is provided
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Error is sent to Sentry when DSN is configured
2. Error is sent to PostHog only when apiKey is present and errorTracking flag is true
3. Error tracking results in a silent no-op when no configurations are provided
4. PostHog analytics middleware is bypassed when autoCapture flag is false
5. CaptureException in analytics.js correctly formats message, type, stack, and requestId for PostHog
6. Verify 'default-safe' invariant: flags default to false when only the apiKey is provided
🗒️ Improve review quality by adding custom instructions
Up to standards ✅🟢 Issues
|
Summary
lib/services/errorTracker.js:captureException(err, ctx)fans out to Sentry (if dsn configured) AND PostHog (if apiKey +errorTracking===true). Silent no-op when neither is configured.analytics.js: newcaptureException(err, ctx)method — sends PostHog$exceptionevent with message, type, stack, requestId.express.js:analyticsMiddlewarenow opt-in viaposthog.autoCapture===true(was always-on).sentry.setupExpressErrorHandlerreplaced byerrorTracker.setupExpressErrorHandler(includes fan-out middleware with userId/requestId context).posthog.errorTracking: falseandposthog.autoCapture: falseopt-in flags — default-safe invariant.Default-safe invariant
With only
posthog.apiKeyset: zero new features activate. All flags default tofalse.Test plan
npm run lint— clean (0 errors, 0 warnings)npm run test:unit— 519/519 tests passingnpm run test:integration— 5 pre-existing failures only (unrelated to this PR, improved from 37 before)