Repository navigation
fix(node): capture unhandled rejections when the host has its own listener - #5472
ablaszkiewicz wants to merge 3 commits into
Conversation
…tener In Node's default throw mode (and warn-with-error-code), posthog-node did not listen for unhandledRejection and relied on Node escalating the rejection to uncaughtException. A host with its own unhandledRejection listener, such as Next.js, stops that escalation, so the rejection was never captured. The SDK now listens for unhandledRejection only while another listener exists. Such a listener has already stopped Node's default handling, so adding ours changes nothing. It tracks hosts that add or remove their listener after init through newListener and removeListener. A rejection captured this way and then rethrown by the host listener is not captured again by the uncaught-exception handler. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FkuJAD3n8f6Ra7L9p2UmPw
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 6907893: 1 Should fix.
This is the only automatic review while this pull request is a draft. Comment @posthog review to review new changes sooner. Otherwise, a review of the new changes runs when you mark it ready for review.
See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.
Coherence0 spec problems · 0 failing chokepoints · 2 spec gaps · 0 guard failures 44 components, 7 bullets (5 invariants, 2 requirements), 0 problems Chokepoints: none declared. Spec gaps: 2 entrances with outside or unknown trust and no traced control, and 10 more held by the adoption baseline
Close one with |
Node SDK acceptance v2Commit: 323c622 Capture v0✅ 55 passed / ❌ 1 non-passing / Drill-down: run artifacts → ❌ migration/yaml-parity-v1/remote-flags-v1.feature::Omitted GeoIP settings default to enabled in flag requests — failed_assertionFlags field differs Code: Failed step: migration/yaml-parity-v1/remote-flags-v1.feature:134 the first flags request field "geoip_disable" should equal JSON false Operation: Field: Expected: false Actual value: true Harness exit: 1; report validation exit: 1. Capture v1✅ 120 passed / ❌ 1 non-passing / Drill-down: run artifacts → ❌ migration/yaml-parity-v1/remote-flags-v1.feature::Omitted GeoIP settings default to enabled in flag requests — failed_assertionFlags field differs Code: Failed step: migration/yaml-parity-v1/remote-flags-v1.feature:134 the first flags request field "geoip_disable" should equal JSON false Operation: Field: Expected: false Actual value: true Harness exit: 1; report validation exit: 1. |
Error tracking smokeCommit: 7/8 passed (0 skipped, 0 uncovered)
|
… host-handled rejections Shares one set of process listeners across clients and bundled copies, with one capture per project. A rethrow, a double delivery or a captureException from a framework hook counts once. Rejections wait a one-second grace period, cancelled by rejectionHandled, and are sent on flush(), waitUntil, shutdown(), a fatal exit and beforeExit. React and Next.js control-flow errors and AbortError rejections are skipped. Our listener is prepended so it sees the host's listeners, and reproduces the runtime's handling when the host's listener disappears without an event (Bun). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FkuJAAYPkxG7cuvaUbEGCf
|
@PostHog review --deep |
|
@ablaszkiewicz PostHog Review is reviewing this pull request. |
PostHog Review · Deep · reviewed
|
| Step | Status | Result |
|---|---|---|
| Prepare the diff | done | 4 files, 1 chunk |
| Pick perspectives | done | 3 perspectives |
| Run review passes | done | 11 issues (+1 blind-spot) |
| Merge overlapping findings | done | 12 → 12 |
| Validate findings | done | 7 kept, 5 dismissed |
| Publish | done | 7 posted (view review) |
…, and send what a flush owes Addresses the review of the host-handled rejection capture: - A deferred capture runs in a snapshot of the async context the rejection happened in, so it reports that request's context and OpenTelemetry span, or none, never those of the code that triggered the capture. - Among clients of one project, the one whose request context is active captures. - captureExceptionImmediate on an occurrence autocapture already captured sends that event before resolving. - A flush also sends what another client of the project captured for the rejections it waited for. - An explicit capture just before a rejection carries into the pending rejection. - One waitUntil keep-alive per client covers every pending rejection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FkuJAAYPkxG7cuvaUbEGCf
Pull requests: 1. 👉 this PR 2. #5473 3. PostHog/context-mill#446 4. PostHog/wizard-workbench#4805 5. PostHog/posthog#125314
Problem
posthog-node's exception autocapture never captured a fire-and-forget promise rejection under Next.js, Nitro (Nuxt) or PM2. In Node's default
throwmode the SDK relies on Node escalating the rejection touncaughtException. A host with its ownunhandledRejectionlistener stops that escalation.Changes
throwandwarn-with-error-codemodes, listen only while the host has a listener of its own, so Node's handling never changes. If the host's listener disappears without an event (Bun), reproduce what the runtime would have done.captureExceptionfrom a framework hook (Nitro) counts once.rejectionHandled: late awaits aren't reported.flush(),waitUntilandshutdown()still send pending rejections. A deferred capture runs in the async context the rejection happened in, so it keeps that request's context and OpenTelemetry span.AbortError, the same set Sentry drops.How did you test this code?
node-unhandled-rejection,next1{4,5,6}-rejection,nuxt-rejection). Each runs without the SDK, with posthog-node 5.59.0 and with this PR.--unhandled-rejectionsmodesnext startand custom server, each with and without PM2The last three columns count PostHog events, except where a cell says otherwise.
once, self-removing, rethrowingAbortErrorKnown limits:
process.exit()in its listener, andstrictmode, exit before any capture.captureExceptionmore than 1 s after a rejection, or a few ms after an uncaught exception, still adds a second event.Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
🤖 Generated with Claude Code
https://claude.ai/code/session_01FkuJAAYPkxG7cuvaUbEGCf