Commit Graph

5 Commits

Author SHA1 Message Date
lefarcen 50843ff149 give the unattributable client failures a real error code (#5960)
* give the unattributable client failures a real error code

Three buckets in the daily stability report name nothing, and in every case the
evidence was already in scope at the emission site:

- BYOK connection test: 801 failures across 201 devices in 24h — 10% of ALL
  connection tests — reported `result.kind` verbatim, so every response the
  daemon's status→kind map had no case for collapsed into `unknown`. The user
  is told "connection failed" and we cannot say whether it was an out-of-credits
  402, a self-signed cert, or a base URL pointing at an HTML login portal.
  `ConnectionTestResponse` already carried `status` and a redacted `detail`;
  only the emission site threw them away.

- Frontend exceptions: the Top-5 table literally renders a row as
  `Error  — 235 次 / ~74 台`. Two independent sources — `buildExceptionList`
  copied `error.name`/`error.message` straight through and both can be '', and
  `parseStack` returned a zero-frame stacktrace for a stackless error, which
  PostHog drops at ingestion so neither `$exception_type` nor
  `$exception_value` survives.

- Export failures (8.1%, 210/24h): everything that was not a sidecar-skew
  message fell through to `err.name`, i.e. the literal "Error".

The daemon side of the same path flattened every sidecar contract violation to
`UPSTREAM_UNAVAILABLE`, which forced the web layer to regex-match English prose
to tell a version-skewed mesh from a dead renderer; it now forwards the
sidecar's real code, and the web classifier treats the remaining generic
envelope codes as non-classifications so a specific message still wins.

Two existing export-code expectations changed on purpose and say why inline: a
generic envelope `.code` no longer outranks message classification, and a
message carrying an HTTP status now reports that status instead of "Error".

* keep the forwarded sidecar code inside ApiErrorCode

Follow-up on the same PR. Forwarding the sidecar contract violation's `code`
compiled, but it was leaving the declared union: `ctx.http.sendApiError` is
typed `(...args: any[]) => any` in server-context.ts, so nothing caught that
`SIDECAR_UNKNOWN_MESSAGE` is not an `ApiErrorCode`. Every web-side handler
typed against that union would have failed to match the code the daemon
actually put on the wire.

Declare the two SIDECAR_* codes in API_ERROR_CODES and narrow the helper's
return type to ApiErrorCode, allowlisting exactly those two so a future sidecar
code cannot silently leave the union again.

No consumer of the export 502 switches on the old code — the UPSTREAM_UNAVAILABLE
call sites are finalize (useFinalizeProject), AMR run guidance, and ChatPane run
errors, none of which are on this route — so this is additive, not a behavior
change for anything that exists today.

* make the attribution actually fire on the production paths

Three review findings, all correct, all verified against the real code paths
rather than the synthetic ones the first cut tested.

1. The sidecar code passthrough was dead. `requestJsonIpc` rejects with
   `new Error(response.error?.message)` — its response type does not even
   declare a `code` — so SIDECAR_UNKNOWN_MESSAGE never reaches the route, and
   the helper always fell back to UPSTREAM_UNAVAILABLE. Removed the passthrough
   (and the two API_ERROR_CODES entries added for it: nothing emits them). The
   message-based classifier already achieves the attribution and is tested;
   dead code that reads as working is worse than no code. Threading the code
   through IPC → API → web export result is a three-layer change and belongs in
   its own PR.

2. The zero-frame guard checked the input instead of the output. A header-only
   stack ("Error: Script error.") is truthy, so it skipped the fallback, and the
   parser then dropped that single line as a message — yielding `frames: []`,
   the exact ingestion failure this change exists to prevent. The guard now runs
   on the PARSED result, covering missing / empty / header-only / unparseable
   stacks, with a fixture matrix over all four.

3. The certificate bucket could not fire in production. undici reports a TLS
   failure as a TypeError whose message is the generic `fetch failed`, with the
   reason only on `cause.code`, and `networkErrorToKind` maps just an allowlist
   — so a self-signed cert became `kind: unknown` + `detail: "fetch failed"`.
   The unit test passed only because it was fed a detail string production never
   emits. The daemon now appends the cause code to `detail`, and the client
   tests assert against that real shape.

* stop a 2xx status from claiming a failure it cannot explain

Review catch, and it made this PR's headline case unreachable. The status
fallback ran before every detail classifier, so the daemon's
`{ kind: 'unknown', status: 200, detail: 'Unexpected token < …' }` — which is
exactly the 'base URL points at an HTML login portal' shape, returned when a 2xx
body fails JSON.parse (connectionTest.ts) — was reported as HTTP_200, and
INVALID_JSON_RESPONSE could never fire.

Reproduced before fixing:
  byokErrorCode({kind:'unknown',status:200,detail:'Unexpected token < …'})
    → 'HTTP_200'

The cut is now at 300 rather than 0. A 2xx that still failed did so for a reason
the status cannot express, so it falls through to the detail classifiers; 3xx
stays on the status side because the daemon fetches with `redirect: 'error'`,
making a redirect a real failure the status does name.

* read the daemon-appended cause code instead of enumerating codes

Review catch on a gap my own previous commit opened: forwarding cause.code from
the daemon put OpenSSL's TLS codes on the wire, and this file could not read
them. UNABLE_TO_GET_ISSUER_CERT_LOCALLY, CERT_NOT_YET_VALID and CERT_HAS_EXPIRED
have no Node-errno shape, and the `\bcertificate\b` fallback never matches a
name that spells it CERT — so the signal was preserved on the wire and dropped
here, landing back in UNKNOWN_NO_SIGNAL.

Read the daemon's known trailing `(CODE)` position directly rather than
extending an allowlist: an enumeration would silently drop every code added
upstream after this file was written, which is the same failure mode one layer
down. Inline codes keep a widened pattern, and the TLS fallback now also matches
CERT-shaped names.

Fixtures cover all seven codes the daemon can now forward.
2026-07-22 06:57:48 +00:00
lefarcen e38f5d1c24 fix(web): stamp chunk_id on exception frames so PostHog can symbolicate (#5679)
* fix(web): stamp chunk_id on exception frames so PostHog can symbolicate

Every packaged front-end exception ships with minified frames (`?:?`,
mangled function names) even though tools-pack faithfully injects chunk
ids and uploads the browser sourcemaps to PostHog on every release. In
production, 0 frames ever carried a chunk id and 5,140 frames in 7 days
failed with "had no source url or chunk id".

Root cause: exception capture is owned by our own error-tracking.ts (it
runs before consent + before posthog-js loads, via a direct ingest
fetch), and client.ts sets `capture_exceptions: false`. The chunk id
that @posthog/cli bakes into each chunk is stamped onto frames by
posthog-js's native capture — which we bypass. Packaged chunks load over
the `od://` scheme, so PostHog cannot fetch a `.map` by URL either; the
chunk id is the ONLY correlation key, and we never attached it. Result:
the uploaded maps had nothing to match against.

Fix: error-tracking.ts now builds the same filename->chunkId map from
`globalThis._posthogChunkIds` that @posthog/core does, and stamps
`chunk_id` on each frame whose chunk registered one. Scrubbing preserves
it (spread + field override), and it is set before scrub, so PostHog can
correlate regardless of filename redaction.

* fix(web): key chunk id on the newest registration frame, not the outermost

PerishCode review: parseStack keeps Error.stack newest-first while
@posthog/core reverses to oldest-first, so copying its length-1..0 loop
selected the wrong end. For a multi-frame registration stack that keyed
the chunk id onto a webpack-runtime filename and left the real chunk
unmatched. Iterate from the newest frame (frames[0]) instead; regression
test now uses a multi-frame registration stack.
2026-07-15 08:10:16 +00:00
lefarcen dab92a8cf2 fix(analytics): stamp frame platform + drop fetch-failure noise in exception transport (#4661)
* fix(analytics): stamp frame platform + drop fetch-failure noise in exception transport

The hand-rolled browser-exception transport in error-tracking.ts (the sole
$exception source since client.ts sets capture_exceptions:false) had two bugs
that PostHog's weekly digest surfaced as "512K exceptions / 511K failed to
ingest":

1. Frames never carried `platform`. PostHog's exception ingestion requires it
   per frame to pick the symbolication/grouping strategy; without it every
   event failed processing with "missing field platform" and was dropped
   server-side. 100% of our ingested $exception events had platform=(missing),
   all from capture_source=web/error-tracking. Now stamped 'web:javascript' on
   every frame, matching posthog-js.

2. `TypeError: Failed to fetch` from the packaged app's daemon connection
   dropping (restart, boot race, navigation/unmount abort, offline) was ~90% of
   all captured exceptions — environmental noise, not bugs, one polling loop can
   emit thousands. Dropped at the capture chokepoint along with AbortError and
   the WebKit/Firefox network-error wordings.

Also hardens the telemetry beacon: `void fetch(...)` had only a synchronous
try/catch, so a failed beacon rejected unhandled and got re-captured as its own
Failed to fetch — a self-amplifying loop. Now `.catch()`-ed.

Red specs in error-tracking.test.ts go red on main (frames lack platform; noise
is dispatched) and green here.

* fix(analytics): scope fetch-noise drop to packaged runtime, stop dropping AbortError

Address review: captureException is the single chokepoint for window.error /
unhandledrejection / reportHandledException, so a blanket drop of
`Failed to fetch` would also suppress a real broken-/api/* or CORS/TLS signal
from the normal web app — and blanket-dropping AbortError was never justified
by the data.

- Gate the fetch-failure drop on the exception originating in packaged app
  code (od:// stack origin), which is where the ~90% daemon-connection churn
  lives. The same TypeError from a web (http/https) context stays captured.
- Remove the AbortError drop entirely (not a measured noise source, and not
  necessarily fetch-lifecycle cancellation).

Test now asserts the invariant directly: packaged od:// fetch noise dropped,
the identical web-origin error kept, AbortError kept.

* test(analytics): cover beacon-failure self-amplification backstop

Address review: add a regression that a failed telemetry beacon, surfacing as
an unhandledrejection from od:// transport code, does not re-enter capture as a
second $exception/beacon. Exercises the packaged-noise backstop (the
loop-breaking layer observable under jsdom; dispatch()'s .catch() is a
process-level promise rejection that jsdom does not route to
window.onunhandledrejection, so it is not unit-observable here).

* fix(analytics): recognize file:// app-bundle frames as packaged origin

Address blocking review: packaged exceptions don't only arrive as od:// frames
— source-mapped frames surface as file:///.../<Channel>.app/Contents/Resources/...
(the shape scrub.ts rewrites). The od://-only check let those packaged
fetch-failures leak through. Broaden originatesInPackagedApp to also match the
.app/Contents/Resources bundle marker (channel-agnostic, so Beta/Preview builds
are covered). Regression added: file:// app-bundle Failed-to-fetch stacks are
dropped too; red against the od://-only check, green here.
2026-06-23 12:41:56 +00:00
elihahhahahah11 3a4ee607f1 fix(web): patch exception-tracking app_version after /api/version resolves (#4508)
The always-on exception tracker is bootstrapped once with whatever app
version is known at boot. When /api/version hasn't resolved yet that is
the '0.0.0' placeholder, and because bootstrapExceptionTracking() and the
analytics client's loaded() callback are both single-flight, the context
stays permanently stamped with '0.0.0' even after the real version lands.
That accounts for the large share of $exception / safety events reporting
app_version=0.0.0 in PostHog.

Add patchExceptionTrackingAppVersion(version) which mutates only the
appVersion field on an existing context, and call it from the
AnalyticsProvider effect right after resolveAppVersionForCapture(). The
effect re-runs whenever the appVersion state transitions from the
placeholder to the daemon-pinned version, so subsequent exception/safety
events carry the real version without re-running the single-flight boot.

The patch is a no-op when no context is set yet, so it can't resurrect a
cleared (no-key) context.

Co-authored-by: free666799 <293857035+free666799@users.noreply.github.com>
2026-06-19 15:08:26 +00:00
lefarcen 88dee44892 feat(analytics): always-on $exception capture with early window hooks (#2521)
PostHog Error tracking was missing the vast majority of real exceptions:

  1. posthog-js's capture_exceptions: true is silenced by opt_out_capturing,
     so every opted-out user vanished from the error feed even though we
     could perfectly safely keep collecting their stacks (the consent
     toggle's user copy gates analytics, not safety telemetry).
  2. posthog-js is dynamically imported only after /api/analytics/config
     resolves AND the user has consented. Errors thrown during the first
     1-2 seconds (React hydration, early effects) had no listener to
     catch them.

Net effect: 14d $exception count was 54 events / 10 users across ~5k DAU,
producing the misleading 99.93% crash-free curve in PostHog's dashboard.

This PR makes exception capture independent of both gates:

  - apps/web/src/analytics/error-tracking.ts (new): own window.error +
    unhandledrejection handlers, in-memory buffer (capped at 50 entries),
    direct fetch to https://<host>/i/v0/e/ with the public phc_ key. Same
    scrub layer as the posthog-js path so file paths still get redacted.
  - apps/web/app/[[...slug]]/client-app.tsx: installErrorHandlers() at
    module-load, before React or any feature code can throw.
  - apps/web/src/analytics/provider.tsx: bootstrapExceptionTracking() in
    the identity useEffect, parallel to getAnalyticsClient() — runs
    regardless of consent state, fetches /api/analytics/config, hands the
    phc_ key + host + distinctId to the error tracker so buffered events
    can flush.
  - apps/web/src/analytics/client.ts: capture_exceptions: false so
    posthog-js stops also emitting $exception (would have produced
    duplicate events server-side); also re-bridges the error-tracking
    context inside the loaded() callback so future events inherit the
    fully-resolved appVersion / sessionId.
  - apps/daemon/src/server.ts + packages/contracts: /api/analytics/config
    now returns key + host even when consent=false. enabled still reflects
    only the analytics consent toggle (posthog-js full autocapture stays
    off when enabled=false), but the always-on error tracker can read key
    directly. Forks without POSTHOG_KEY still get key=null and the whole
    pipeline becomes a no-op — fork-safe by construction.
  - apps/web/src/analytics/scrub.ts: regex fix so packaged-mac paths like
    /Applications/Open Design.app/Contents/Resources/apps/web/... (which
    contain a space) get fully rewritten to app://apps/web/...; previously
    the [^\s] guard stopped at 'Open' and leaked the install dir.

Validation:

  - pnpm --filter @open-design/web typecheck: pass
  - pnpm --filter @open-design/web test: 199 files / 1823 tests pass
    (includes 8 new error-tracking.test.ts cases for buffer cap, hook
    install, scrub, and direct dispatch)
  - pnpm --filter @open-design/daemon test: 250 files / 2971 tests pass
  - pnpm guard: pass

After release/v0.8.0 ships and rolls out, expect the crash-free curve to
drop from the artificial 99.93% to a realistic 95-98% — that's not a
regression, it's the first time we're measuring it.
2026-05-21 13:07:26 +08:00