3 Commits

Author	SHA1	Message	Date
lefarcen	d00492ca2a	fix(desktop,web): stop 10-minute export hangs and make export failures attributable (#7182) ... * fix(desktop,web): stop 10-minute export hangs and make export failures attributable Three findings from PostHog on `artifact_export_result` (14d, image format): 13.5% of image exports fail, 22.8% on 0.19.2, and 20 users failed 3-11 times each with zero successes. All failures are packaged desktop (od://). 1. waitForPrintableContent had no timeout. It awaits document.fonts.ready, every <img> load/error, and a new Image() per CSS url() background — under od:// any of those can settle neither way, leaving the promise pending until the daemon's 600s desktop-IPC ceiling. 122 of 142 DESKTOP_RENDERER_UNAVAILABLE failures sat at ~10 minutes before the user saw an error. Its sibling waitForPrintReadyHandshake already guards itself with a 30s race for this exact reason; this one was missed. Bound it in-page per resource (10s) with an outer backstop (15s), resolving rather than rejecting: capturing without a late resource beats failing ten minutes later, and the script already treats a failed resource as fine. 2. error_code reported the literal "Error" for 148 failures / 82 users — 48% of them, and undiagnosable. The daemon classifies its own errors, but the web export path kept only .error.message and dropped .code, so exportErrorCode fell through message-regex matching to err.name. Carry code + status through, prefer a meaningful err.name (TypeError etc), fall back to the daemon envelope code, and report UNCLASSIFIED instead of "Error". 3. CAPTURE_FAILED (90 events / 37 users) collapsed five distinct give-up points into one code. Tag each stage so the bucket says which step of the fallback chain gave up. * fix(desktop): bound the artifact document load and release the renderer End-to-end verification against a real Electron main process found the first fix incomplete: the export never even reached waitForPrintableContent. Probing the actual hang showed [OD-PROBE] loadURL start <- and nothing after it renderDeckSlides awaited window.loadURL() directly, and Electron resolves that on did-finish-load, which Chromium fires only once EVERY subresource has settled. One image or font URL that answers neither way leaves it pending forever, so the export hung one step earlier than the function I had bounded. Wait on dom-ready instead (the capture pipeline only needs a parsed, scriptable document; subresources are waitForPrintableContent's job and it bounds itself), with a 15s backstop and the loadURL rejection kept handled. Bounding the waits still was not enough. The requests we stop waiting for stay in flight and every later executeJavaScript queues behind a renderer still busy with them, so the steps after the wait still cost ~38s and the total stayed unstable. Cancelling the outstanding loads on timeout hands the renderer back. Measured on an artifact whose <img> and CSS url() both point at a socket that accepts and never answers, against a live daemon+desktop runtime: before hung past the daemon's 600s IPC ceiling bound loadURL 169s -> 53s, but unstable (a later run hung again) + stop() 25.7s / 25.8s / 25.7s, identical output every run stage breakdown without stop() with stop() loadMs 89 70-160 assetsMs 15001 15001 prepareMs 15340 6 renderMs 22459 10566 totalMs 52889 25700 Both Electron mocks that stub webContents grew the once() the real EventEmitter always had; without it the new dom-ready wait threw and base-href-precedence went red. * fix(desktop): address review — propagate load failures, cancel on either bound Two blocking findings from review, both real and both verified by reverting each fix and watching the matching [P0] go red. 1. A did-fail-load was being swallowed. `.catch(() => undefined)` turned a genuine main-document failure into an immediately-resolved race winner, so the pipeline went on to capture Chromium's error page and could report a successful-but-wrong export. That is worse than the hang this function exists to prevent: the previous bare `await window.loadURL(...)` rejected and renderDeckSlides reported RENDER_FAILED. Race tagged outcomes instead — a failure that beats dom-ready rethrows, while one arriving after dom-ready has already won stays handled (onRejected is attached up front) rather than escaping as an unhandled rejection. 2. Cancellation keyed off the outer timer only. The in-page per-resource deadline is 10s and the outer bound is 15s, so whenever the renderer is healthy the script returns on time, `timedOut` stays false, and the abandoned requests were left in flight — exactly the state that makes every later executeJavaScript crawl. My own measurement hid this: assetsMs 15001 means the outer timer won there. The injected script now counts the deadlines it fired and reports `{ stalled }`, and either layer giving up triggers stop(). Re-verified end-to-end against a live daemon + desktop runtime on the same artifact (an <img> and a CSS url() both pointing at a socket that accepts and never answers): 25.9s / 25.7s / 25.7s, http=200, byte-identical PNG, stage split loadMs 69 / assetsMs 15002 / prepareMs 8 / renderMs 10549.	2026-08-21 07:54:41 +00:00
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	1eb3898795	fix(web): report specific export error_code instead of generic "Error" (#5220) ... * fix(web): report specific export error_code instead of generic "Error" `artifact_export_result.error_code` reported `err.name`, which collapses to the generic "Error" for the common plain-Error case — hiding distinct export failure modes in analytics. Most notably the daemon↔desktop sidecar version skew (a freshly-updated daemon sends the 0.13.0 `render-slides` message to an older desktop process that rejects it as "unknown desktop sidecar message") was indistinguishable from ordinary render failures. Classify the thrown error into a stable, queryable code (a structured `.code` wins; otherwise map the known daemon signatures): DESKTOP_SIDECAR_UNKNOWN_MESSAGE for the version skew, DESKTOP_RENDERER_UNAVAILABLE for a plain unreachable renderer, else the error name. This lets PostHog separate the version-skew population from other failures and track whether a fix lands. * fix(web): classify whole-deck image export failures too handleImageExportSave's catch still reported err.name, so whole-deck image exports (which go through the off-screen desktop renderer and can hit the same render-slides version-skew error) stayed generic 'Error'. Route them through exportErrorCode too, closing the image slice of the pdf/pptx/image failure population. The classifier is format-agnostic and unit-tested, so image reuses the same covered logic.	2026-07-06 14:19:17 +00:00