fix(observability): use posthog-node captureException SDK method for UI grouping#3659
Conversation
…UI grouping
Replaces legacy client.capture({event: '\$exception', props}) with SDK
client.captureException(err, distinctId, additionalProperties) so PostHog
auto-populates \$exception_list + \$exception_fingerprint — required by the
Error Tracking UI for grouping/display. Fixes #3658.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3659 +/- ##
=======================================
Coverage 89.20% 89.20%
=======================================
Files 136 136
Lines 4668 4669 +1
Branches 1451 1452 +1
=======================================
+ Hits 4164 4165 +1
Misses 392 392
Partials 112 112
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR updates the analytics error-reporting path to use the PostHog Node SDK’s native captureException() API so PostHog Error Tracking can populate $exception_list / $exception_fingerprint and properly group/display exceptions in the UI (fix for #3658).
Changes:
- Switch
AnalyticsService.captureException()from legacycapture({ event: '$exception', ... })toclient.captureException(err, distinctId, additionalProperties). - Update unit/integration tests to assert against
captureExceptioncalls (and add arequestIdassertion). - Keep existing no-op/safety behavior (no client / swallow SDK errors) while aligning payload shape with PostHog Error Tracking expectations.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| lib/services/analytics.js | Replace legacy $exception capture with SDK captureException and pass additional properties for PostHog Error Tracking UI grouping. |
| lib/services/tests/analytics.captureException.unit.tests.js | Update expectations to validate captureException(err, distinctId, props) behavior and include a requestId property test. |
| lib/services/tests/analytics.service.unit.tests.js | Update service-level tests to assert captureException calls and error-swallowing behavior via the new SDK method. |
| lib/services/tests/logger.posthog.transport.integration.tests.js | Update logger→PostHog integration assertions to validate that errors flow through SDK captureException. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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 (3)
WalkthroughThe PR migrates exception capture in the analytics service from manually emitting ChangesPostHog SDK captureException Migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@lib/services/analytics.js`:
- Around line 176-181: The current construction of additionalProperties lets
ctx.properties.requestId override ctx.requestId because ...(ctx.properties ??
{}) is spread after the explicit requestId key; update additionalProperties so
that the authoritative requestId (ctx.requestId) wins by moving the requestId
assignment after the spread of ctx.properties (or by spreading ctx.properties
first and then adding requestId: ctx.requestId and finally ...explicit),
referencing the additionalProperties object and the ctx.requestId /
ctx.properties symbols to locate the change.
In `@lib/services/tests/logger.posthog.transport.integration.tests.js`:
- Around line 37-49: The test currently asserts payload shape but not
uniqueness; add an explicit call-count assertion to ensure a single emission by
verifying mockPostHogInstance.captureException was called exactly once (use
expect(mockPostHogInstance.captureException).toHaveBeenCalledTimes(1)) in the
test 'logger.error(message, { error }) emits a single $exception event via SDK
captureException' after invoking logger.error and before/after the existing
toHaveBeenCalledWith assertion so duplicate emits fail the test.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 946b80d2-ac22-4bb9-90d0-692831cf1f8c
📒 Files selected for processing (4)
lib/services/analytics.jslib/services/tests/analytics.captureException.unit.tests.jslib/services/tests/analytics.service.unit.tests.jslib/services/tests/logger.posthog.transport.integration.tests.js
CodeRabbit findings on PR #3659: - captureException additionalProperties construction let ctx.properties.requestId override ctx.requestId. Move requestId spread after ctx.properties so the explicit ctx.requestId wins. - logger.posthog.transport.integration.tests.js asserted payload shape but not call count. Add toHaveBeenCalledTimes(1). +1 unit test for ctx.requestId precedence.
PierreBrisorgueil
dismissed
coderabbitai[bot]’s stale review
Findings addressed: requestId precedence + toHaveBeenCalledTimes(1) — see commit message
Summary
client.capture({event: '$exception', props})with SDK nativeclient.captureException(err, distinctId, additionalProperties)$exception_list(with parsed stack frames) and$exception_fingerprint— required by the Error Tracking UI for grouping/display$exception_list = null(legacy format not recognized by UI)Closes #3658. Plan ref:
infra/docs/superpowers/plans/2026-05-11-posthog-observability-wave4.md.Files changed
lib/services/analytics.js—captureException()implementation (5 lines)lib/services/tests/analytics.captureException.unit.tests.js— assertions shifted tocaptureExceptionmock, +1 newrequestIdtestlib/services/tests/analytics.service.unit.tests.js— same assertion shiftlib/services/tests/logger.posthog.transport.integration.tests.js— same assertion shiftTest plan
npm run lint— cleannpm run test:unitanalytics path — 145 tests pass (7 in captureException suite, 4 in logger transport)npm run test:coverage— 1785 pass, only pre-existingbilling.extraBalance.listLedger.perfMongoDB failure (unrelated$sortArray)Summary by CodeRabbit
Bug Fixes
Tests