Repository navigation
feat(next): tracing-header identity, request properties and a proxy wrapper for onRequestError - #5473
Draft
ablaszkiewicz wants to merge 1 commit into
Draft
ablaszkiewicz wants to merge 1 commit into
ablaszkiewicz wants to merge 1 commit into
Conversation
…rapper for onRequestError onRequestError now takes the distinct and session ids from the x-posthog-distinct-id and x-posthog-session-id request headers first, then from the PostHog cookie, and else captures a personless exception. Header values are sanitized analytics context only. The cookie session id is used only while the session is active: at most 30 minutes since its last activity and 24 hours since it started. To support this, @posthog/core's parsePostHogCookie also returns the $sesid timestamps. The exception also gets $current_url, $request_method, $request_path and $user_agent, all without the query string. withPostHogProxy wraps a proxy (or middleware) function, captures the errors it throws through the onRequestError path, and rethrows them. Next.js 16 does not report proxy errors to onRequestError. Next.js 15 and earlier do report edge middleware errors, so the wrapper marks the errors it captures and onRequestError skips marked errors. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FkuJAD3n8f6Ra7L9p2UmPw
Contributor
|
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 8b91070: 4 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.
| // Capture failures are already reported; the proxy error must still reach Next.js. | ||
| } | ||
| // Next.js 15 and earlier also report edge middleware errors to onRequestError, which skips marked errors. | ||
| markErrorAsCaptured(error) |
Contributor
There was a problem hiding this comment.
P2 · Mark proxy errors only after capture succeeds
When middleware capture fails after a transport error, captureNextRequestError catches the failure and returns, but captureProxyError still sets the captured marker. On Next.js 15, the subsequent onRequestError then skips an exception that was never delivered. Propagate the capture outcome and mark the error only after successful capture.
Comment on lines
305
to
+306
| const record = headers as Record<string, HeaderValue> | ||
| return normalizeHeaderValue(record[name] ?? record[name.toLowerCase()]) | ||
| return record[name] ?? record[name.toLowerCase()] |
Contributor
There was a problem hiding this comment.
P2 · Resolve tracing-header record keys case-insensitively
With plain-record headers such as X-PostHog-Distinct-Id, getTracingHeader passes a lowercase name, making these two lookups identical. It ignores the supplied identity and falls back to the cookie or personless capture, violating the tracing-headers contract required by AGENTS.md:11; compare record keys case-insensitively.
Comment on lines
+25
to
+28
| if (!isNextControlFlowError(error)) { | ||
| await captureProxyError(error, args[0], options, captureRequestError) | ||
| } | ||
| throw error |
Contributor
There was a problem hiding this comment.
P2 · Bound capture latency before rethrowing proxy errors
When the ingestion endpoint stalls, wrapProxy waits for captureExceptionImmediate before rethrowing. Default transport settings allow four 10-second attempts plus three 3-second delays, holding the failed request for roughly 49 seconds and risking a platform timeout before Next.js handles the original error. Use waitUntil where available or impose a short capture deadline.
| import type { NextRequestErrorContext, NextRequestErrorRequest, OnRequestErrorOptions } from './onRequestError.types.js' | ||
|
|
||
| /** The arguments Next.js passes to a proxy (middleware) function: the request, then for example the fetch event. */ | ||
| export type NextProxyArgs = [request: Request, ...rest: unknown[]] |
Contributor
There was a problem hiding this comment.
P2 · Contextually type proxy callbacks with Next.js arguments
When an inline callback reads request.nextUrl or calls event.waitUntil, withPostHogProxy contextually types its arguments from NextProxyArgs as Request and unknown. Standard Next.js proxy operations therefore fail type checking unless callers manually annotate the arguments. Use NextRequest and NextFetchEvent for contextual typing.
Contributor
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 |
Contributor
Contributor
Node SDK compliance v2Commit: 8b91070 Capture v0 — acceptance✅ 28 passed / ❌ 3 non-passing / Drill-down: run artifacts → ❌ server_snapshot_payload_false — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_zero — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_null — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: Harness exit: 1; report validation exit: 1. Capture v1 — acceptance✅ 28 passed / ❌ 3 non-passing / Drill-down: run artifacts → ❌ server_snapshot_payload_false — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_zero — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_null — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: Harness exit: 1; report validation exit: 1. Capture v0 — migration✅ 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 — migration✅ 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. |
Contributor
|
Size Change: +6.32 kB (+0.03%) Total Size: 25.3 MB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
posthog-js Compliance ReportDate: 2026-10-28 21:07:27 UTC ✅ All Tests Passed!26/26 tests passed Capture Tests✅ 26/26 tests passed View Details
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull requests: 1. #5472 2. 👉 this PR 3. PostHog/context-mill#446 4. PostHog/wizard-workbench#4805 5. PostHog/posthog#125314
Problem
onRequestErrorfrom@posthog/nextfalls short in three ways:X-POSTHOG-*tracing headers (posthog-jstracing_headers) is captured under the wrong person or none. A stale cookie session links the exception to an old session replay.$current_url,$request_methodor$request_path.proxy.tserrors never reach it on Next.js 16. The proxy wrapper reports throughedgeInstrumentationOnRequestError(next/dist/build/templates/middleware.js). That readsglobalThis._ENTRIES.middleware_instrumentation, an edge-only registry, and since Next.js 16 the proxy runs in Node.js only.Found in an end-to-end test of the wizard's Next.js error tracking.
Changes
x-posthog-distinct-id/x-posthog-session-idheaders win, then the cookie, then a personless capture. Header values are trimmed, stripped of control characters and capped at 1000 characters, as the sdk-specstracing-headersspec requires. They are analytics context only.@posthog/core:PostHogCookieStategains two optional fields,sessionLastActivityTimestampandsessionStartTimestamp. Other callers are unaffected.$current_url(from thehostheader and the path; protocol fromx-forwarded-proto, elsehttps),$request_method,$request_pathand$user_agent, all without the query string. The existing$http_method,$pathnameandnextjs_*properties stay.withPostHogProxy(proxy, options?): wraps a proxy or middleware function, takes the same options asonRequestError, captures what it throws withrouteType: 'proxy', and rethrows. It skips Next.js redirect and not-found errors. It is exported from the Node and edge entries.onRequestError. The wrapper marks what it captures with the existing non-enumerable__posthog_previously_captured_errormarker, and the request-error capture skips marked errors. AWeakSetwould not work, because Next loads separate copies of the module per bundle.withPostHogProxyand the two cookie-state fields are new public API: their shape is open for review.How did you test this code?
@posthog/next: 299 tests passing across 20 files, including the packaging tests after a build.@posthog/core: 2321 passing. posthog-node: 1308 passing.check-types(core, node, next) and lint pass.packages/node/references/posthog-node-references-latest.jsonis regenerated forcheck:public-api; the only change is the two new fields.@posthog/next0.10.1-qa.1,@posthog/core1.61.3-qa.1, posthog-node5.59.1-qa.2) in a Next.js 16.4.2 production app using onlyexport { onRequestError } from '@posthog/next'andwithPostHogProxyaroundproxy.ts. Six request scenarios arrived in PostHog with the expected distinct ID,$session_idand request properties: a Server Component render on navigation,generateMetadata, a route handler called with tracing headers, a route handler called from the page, a server action, and aproxy.tsthrow.Not added:
$ip.Release info Sub-libraries affected
Libraries affected
Also
@posthog/core(minor), which the list above does not include.Checklist
If releasing new changes
pnpm changesetto generate a changeset file (core-cookie-session-timestamps,next-request-error-identity,next-proxy-error-capture)🤖 Agent context
Autonomy: Human-driven (agent-assisted)
@posthog/core's cookie parser rather than a second parser in@posthog/next, so other SDKs reading the cookie can use them.🤖 Generated with Claude Code
https://claude.ai/code/session_01FkuJAD3n8f6Ra7L9p2UmPw