Files
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

125 lines
6.4 KiB
TypeScript

import { describe, expect, it } from 'vitest';
import { exportErrorCode } from '../../src/analytics/export-error-code';
describe('exportErrorCode', () => {
it('classifies the daemon↔desktop sidecar version skew from its wrapped message', () => {
// The exact string the daemon surfaces when a freshly-updated daemon sends
// `render-slides` to an older desktop that predates the message. Before this
// helper it was reported as the generic "Error", hiding the skew in analytics.
const err = new Error(
'desktop renderer unavailable: unknown desktop sidecar message: render-slides',
);
expect(exportErrorCode(err)).toBe('DESKTOP_SIDECAR_UNKNOWN_MESSAGE');
});
it('classifies a plain renderer-unavailable failure separately from the skew', () => {
expect(exportErrorCode(new Error('desktop renderer unavailable: connection refused'))).toBe(
'DESKTOP_RENDERER_UNAVAILABLE',
);
});
it('prefers a SPECIFIC structured .code over message classification', () => {
const err = Object.assign(new Error('desktop renderer unavailable: something else'), {
code: 'SIDECAR_UNKNOWN_MESSAGE',
});
expect(exportErrorCode(err)).toBe('SIDECAR_UNKNOWN_MESSAGE');
});
it('does not let a generic envelope .code outrank message classification', () => {
// This case used to assert the opposite. `UPSTREAM_UNAVAILABLE` is the
// daemon's catch-all envelope for the whole 502 branch, not a
// classification — letting it win meant the version-skew fingerprint in the
// message was thrown away in favour of a code that says nothing. The daemon
// now forwards the sidecar's real code (see
// apps/daemon/src/import-export-routes.ts), so a specific code still wins;
// only the generic wrappers defer to the message.
const err = Object.assign(
new Error('desktop renderer unavailable: unknown desktop sidecar message: render-slides'),
{ code: 'UPSTREAM_UNAVAILABLE' },
);
expect(exportErrorCode(err)).toBe('DESKTOP_SIDECAR_UNKNOWN_MESSAGE');
});
it('falls back to the error name for unclassified failures', () => {
expect(exportErrorCode(new TypeError('boom'))).toBe('TypeError');
// Was asserted as 'Error' — a message carrying an HTTP status is now
// reported as that status, which is strictly more attributable than the
// name of the base Error class.
expect(exportErrorCode(new Error('export request failed (500)'))).toBe('HTTP_500');
// A genuinely signal-free message is now reported as UNCLASSIFIED rather
// than as the base class name. See the [P0] case below for why: "Error"
// was the largest failure bucket in production and identified nothing.
expect(exportErrorCode(new Error('export request failed'))).toBe('UNCLASSIFIED');
});
it('returns UNKNOWN for non-Error throwables', () => {
expect(exportErrorCode('nope')).toBe('UNKNOWN');
expect(exportErrorCode(undefined)).toBe('UNKNOWN');
});
});
/**
* Export failures run at 8.1% (210 in 24h across 48 devices). Before these
* branches every capture-stage and transport failure fell through to
* `err.name`, which is the literal string "Error" for anything thrown as a
* plain Error — the single largest export bucket named nothing at all.
*/
describe('exportErrorCode — non-sidecar failures', () => {
it('does not let a generic daemon envelope code outrank message classification', () => {
// The daemon wraps unrelated failures in BAD_REQUEST/INTERNAL. Those are
// envelopes, not classifications, so the message must still win.
const err = Object.assign(new Error('export timed out after 60s'), { code: 'BAD_REQUEST' });
expect(exportErrorCode(err)).toBe('TIMEOUT');
});
it('separates the capture-stage failures from each other', () => {
expect(exportErrorCode(new Error('nothing was captured'))).toBe('EMPTY_CAPTURE');
expect(exportErrorCode(new Error('canvas is not available'))).toBe('CANVAS_UNAVAILABLE');
expect(exportErrorCode(new Error('unreadable response'))).toBe('UNREADABLE_RESPONSE');
expect(exportErrorCode(new Error('invalid data url'))).toBe('INVALID_DATA_URL');
expect(exportErrorCode(new Error('download failed'))).toBe('DOWNLOAD_FAILED');
});
it('names transport failures instead of leaving them as "Error"', () => {
expect(exportErrorCode(new Error('fetch failed'))).toBe('NETWORK');
expect(exportErrorCode(new Error('getaddrinfo ENOTFOUND host'))).toBe('NETWORK');
expect(exportErrorCode(new Error('ETIMEDOUT'))).toBe('TIMEOUT');
expect(exportErrorCode(new Error('429 Too Many Requests'))).toBe('RATE_LIMITED');
expect(exportErrorCode(new Error('unauthorized'))).toBe('FORBIDDEN');
});
it('falls back to the HTTP status when the message only carries one', () => {
expect(exportErrorCode(new Error('render service replied 503'))).toBe('HTTP_503');
});
// This used to assert `.toBe('Error')`, treating `err.name` as an acceptable
// last resort. Production disagreed: over 14 days, `error_code: "Error"` was
// the single largest image-export failure bucket — 148 events across 82
// users, 48% of all failures — and it identifies nothing, so none of those
// users' failures could be diagnosed. An unclassified failure must say so.
it('[P0] never reports the useless literal "Error" for an unclassified failure', () => {
expect(exportErrorCode(new Error(''))).toBe('UNCLASSIFIED');
expect(exportErrorCode(new Error('something we have no pattern for'))).toBe('UNCLASSIFIED');
expect(exportErrorCode('a string throw')).toBe('UNKNOWN');
});
// The daemon always classifies its own failures; the web export path used to
// drop that code on the floor and keep only the message. When no message
// pattern matches, the envelope code is coarse but real, and beats guessing.
it('[P0] falls back to the daemon envelope code rather than discarding it', () => {
const err = Object.assign(new Error('screenshot render failed'), { code: 'INTERNAL' });
expect(exportErrorCode(err)).toBe('ENVELOPE_INTERNAL');
});
it('reports a status carried on the error, not just one found in the message', () => {
const err = Object.assign(new Error('screenshot render failed'), { status: 502 });
expect(exportErrorCode(err)).toBe('HTTP_502');
});
it('keeps preferring a specific daemon code over the envelope fallback', () => {
const err = Object.assign(new Error('whatever'), { code: 'RENDERER_BUSY' });
expect(exportErrorCode(err)).toBe('RENDERER_BUSY');
});
});