T1: lifecycle events listener and ReportHandler.after

[krassx]

Review-only PR. This change is already on main (the repo works directly on main; see CLAUDE.md). The base branch is the commit it forked from, so the diff below is exactly this task. Merging this PR changes nothing. Review findings become follow-up commits on main. Plan: docs/design/mobile-api-alignment.md.

Mobile parity: Android setLifecycleEventsListener / iOS bugseeLifecycleEvent.

• ReportHandler.after was typed but never invoked; it now runs once the pipeline settles (success or failure), is skipped for a vetoed report, and a throwing handler goes to onError.
• New setLifecycleEventsListener + LifecycleEvents (Android names). Emitted: Launching/Launched/Stopping/Stopped, assembly and upload events, BeforeReportShown/AfterReportShown from the dialog. Blackout events are wired in a later commit; RelaunchedAfterCrash is defined but not emitted.

Review focus / known gaps

• BeforeReportUploaded can fire slightly before the bundle is queued (no hook between assembly and queueing).
• @bugsee/webview: a repeat launch() could replace the host's listener (left, blocking needs that package's refusal-list tests changed).
• Mutator loop was not run on the report-dialog change.

🤖 Generated with Claude Code

[cursor]

Stale comment

Deep review: T1 lifecycle events + ReportHandler.after

Reviewed 0a3f126 against the surrounding report pipeline (submitReport, createTriggerPipeline, durable recover / marker rebuild / native-crash recovery), the WebView restricted/inert surfaces, and the host-boundary helper this repo already uses for callbacks (neverThrow). Findings below were re-checked against the current code; the async-throw path was reproduced.

Overall risk: Medium
Merge recommendation: Request changes — the P1 should land before this is treated as done. The P2s are observation/API holes in the new public surface, not upload-loss bugs.

Most important to fix

1. P1 — Lifecycle listener and ReportHandler.after only catch synchronous throws. An async callback that rejects becomes an unhandledRejection (Node process crash under this SDK's Wave 2.5 disposition). onError is never called. The JSDoc claim that a throwing observer “never affects the SDK / never breaks reporting” is false for the idiomatic case.
2. P2 — Assembly/upload events and after are wired only on the live assemble() + submitReport.onSettled path. Crash recovery, native-crash harvest, WebView, and Electron renderer deliveries are silent.
3. P2 — setLifecycleEventsListener / emitLifecycleEvent were added to public BugseeClient without updating @bugsee/webview's repeat-launch refusal list or inert client (the PR already flags the listener-replace half).
4. P2 — KILL_SDK turns the client off (enterKillState) with no Stopping/Stopped.

Residual / accepted (not blocking)

• BeforeReportUploaded firing inside assemble() before enqueue is a real sequencing lie, but it is documented in the PR body.
• RelaunchedAfterCrash / blackout events are defined and not emitted, as stated.
• Trigger-queue overflow (maxQueueDepth) also produces no lifecycle event; under default logException rate-limit it is hard to hit, but detection/upload() bypass the limiter. Folded into finding 2.

Positive

The live path is carefully done: Launching-before-providers / Launched-after, Stopping-before-drain / Stopped-after, veto skips both events and after, permanent vs retained classification matches isUploadSettled policy (permanent wins over retained), throwing sync observers are isolated, and the browser dialog order (BeforeShown → assemble/upload → AfterShown) is pinned end-to-end. Android wire names are right. after is finally invoked, which was the original hole (docs/review/core-B-capture-pipeline.md §11).

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

P1 High — async listener rejection is an unhandledRejection; onError never runs

emit only try/catches a synchronous throw. An async listener (or any callback that returns a rejected thenable) is the idiomatic JS form of “throwing”, and it bypasses this guard entirely.

Impact: The new public API documents “a throwing listener is routed to onError and never affects the SDK.” On Node this SDK already treats an unhandled rejection as a host process crash (never-throw.ts:27-31, Wave 2.5). A listener that does I/O and fails takes the app down; onError is silent.

Scenario (reproduced):

client.setLifecycleEventsListener(async (type) => {
  if (type === LifecycleEvents.Launched) await fetch('/telemetry');
});
client.launch();

If fetch rejects, this emit returns normally, then one tick later the host sees unhandledRejection. Probe against this file: onError=[], unhandled=['async listener boom'].

The same hole exists for ReportHandler.after (client.ts:742-746, also a raw try/catch). Vitest probe: logException with after: async () => { throw new Error('async after boom') } — onError not called, unhandledRejection fired.

Fix: Route both host callbacks through neverThrow (already exported from this package, already handles thenables). Add tests that assert an async throw is reported to onError and does not produce unhandledRejection — the existing tests only cover () => { throw ... }.

[cursor]

P1 High — same async hole as the lifecycle emitter

after is a host-facing callback whose contract is “a throwing handler goes to onError and never breaks reporting” (filters.ts:55-56). This try/catch only covers a sync throw. after: async () => { throw ... } or after: () => Promise.reject(...) schedules an unhandled rejection after logException has already resolved { ok: true }.

Reproduced with a vitest probe against this onSettled path: onError not called; unhandledRejection fired.

Fix: neverThrow(() => filters.report?.after?.(handled), onErrorSafe) (or equivalent). Snapshot after at submit time if you also want the handler that ran before to be the one that runs after — today a setReportHandler between submit and settle silently drops the in-flight after.

[cursor]

P2 Medium — upload/assembly events and after only fire on the live submitReport × assemble() path

onSettled is attached only here, and upload events are further gated on assembledReports, which is added only in the default assemble() closure (:532). Anything that delivers a report some other way is invisible to the new public observer.

Concrete paths that skip both the lifecycle events and after:

1. Durable recovery / next-launch retry. capture-recovery.ts marker rebuild and durable-upload-pipeline recover()/pump() call uploadPipeline.enqueue directly. A report that emitted ReportUploadFailedWithFutureRetry (or crashed mid-upload) can succeed on the next launch with no AfterReportUploaded. An app that keys a “crash pending” UI on these events never clears it.
2. Native-crash harvest (native-crash-recovery.ts:137) — same: enqueue only.
3. Custom trigger pipelines that never call assemble. @bugsee/webview (webview-report-pipeline.ts:46-62) and Electron renderer (renderer-report-pipeline.ts:48) always resolve {ok:true} (or {ok:false}) without going through assemble(). after does run (this onSettled still fires) but assembledReports.has is false, so a WebView/Electron-renderer listener never sees BeforeReportAssembled / AfterReportUploaded even when the incident was handed to native/main. Detected by the existing test “emits no upload events when an injected trigger pipeline never assembles” — that test pins the gap rather than a WebView/Electron event.
4. Trigger-queue overflow (trigger-pipeline.ts:79-81). report() resolves {ok:false} without calling assemble. Detection and upload() bypass the logException rate limiter, so a burst of unhandled rejections while one assemble is in-flight can drop reports with after running and no lifecycle event. The caller of detection is void submitReport(...), so the {ok:false} is discarded too.

Fix: Emit outcome events (and run after) from a single place that every delivery path shares — at minimum wrap durable enqueue / recover so a settled replay still produces AfterReportUploaded / ReportUploadFailed, and for WebView/Electron emit an honest handoff event (or AfterReportUploaded if you are willing to mean “handed off” rather than “HTTP PUT completed”). For overflow, emit ReportUploadFailed (or ReportAssemblyFailed) even when assembledReports is empty.

[cursor]

P2 Medium — emitLifecycleEvent is public BugseeClient API; WebView repeat-launch neither refuses it nor no-ops it

This is documented as a platform-tier hook (report dialog), but it is on the user-facing client every launch() returns — including @bugsee/webview, where export type Bugsee = BugseeClient.

restrictedClientFor (packages/webview/src/launch.ts:721-745) blocks setReportHandler / filters / stop / the container, then return { ...client, ...blocked }. It does not block setLifecycleEventsListener or emitLifecycleEvent, so a page-script BugseeWebView.launch('anything') keeps the real methods:

• setLifecycleEventsListener(fn) replaces the host bootstrap's listener (the gap the PR body already calls out).
• emitLifecycleEvent(AfterReportUploaded, { reportId: 'fake' }) injects a spoofed event into the host's still-installed listener without replacing it. That half is not mentioned in the PR.

The inert-client fallback (launch.ts:760-781) is worse: the new methods are simply missing. That file's own test comment says a missing method “is the TypeError this fallback exists to prevent.” inert.setLifecycleEventsListener(() => {}) throws in the host page.

Fix: Keep emitLifecycleEvent off the public client (pass it to the dialog as a closed-over function, as launch.ts already does). On WebView, add both methods to blocked (refuse) and to the inert surface (noop). Extend the refusal-list test so deleting them from blocked fails the way deleting setReportHandler already does.

[cursor]

P2 Medium — Stopped is only emitted from stop(); KILL_SDK leaves the SDK dead with no lifecycle event

Stopping/Stopped are the new way to observe session lifetime. The other path that sets launched = false is enterKillState (collector 99099, client.ts:655-665): it halts capture, does not drain, and emits nothing. track() runs enterKillState on the same promise before onSettled, so the app may see ReportUploadFailed and then a client with isLaunched() === false and no Stopped.

Scenario: Token switched off at the collector. App keys cleanup / “SDK down” UI on LifecycleEvents.Stopped (the documented milestone). Capture is already a no-op (killed), stop() later is a no-op (if (!launched) return true — no events). The listener never fires.

Fix: Emit Stopping then Stopped from enterKillState when it actually transitions from launched (or a dedicated event if you do not want kill to look like a user stop()). Also attach a .catch on this drainPending().then(...) so a rejecting flush cannot leave Stopping without Stopped — today's upload flush is allSettled-based and should not reject, but this .then has no rejection path.

[cursor]

Deep review: T1 lifecycle events + ReportHandler.after

Re-reviewed a7e5240 (feature commit still 0a3f126; the new commit only retriggers CI). Surrounding pipeline: submitReport / createTriggerPipeline, durable recover() / marker rebuild / native-crash harvest, WebView + Electron renderer trigger pipelines, @bugsee/webview restricted/inert surfaces, and neverThrow. All five prior findings still hold; one new pairing bug on stop() was reproduced.

Overall risk: Medium
Merge recommendation: Request changes — the P1 should land before this is treated as done. The P2s are observation/API holes in the new public surface, not upload-loss bugs.

Most important to fix

1. P1 — Lifecycle listener and ReportHandler.after only catch synchronous throws. An async callback that rejects becomes an unhandledRejection (Node process crash under this SDK's Wave 2.5 disposition). onError is never called. Reproduced again on this head.
2. P2 (new) — stop(); launch() without awaiting drain emits Stopped after the new session's Launched, while isLaunched() === true. Reproduced: [Launching, Launched, Stopping, Launching, Launched, Stopped].
3. P2 — Assembly/upload events and after are wired only on the live assemble() + submitReport.onSettled path. Crash recovery, native-crash harvest, WebView, and Electron renderer deliveries are silent.
4. P2 — setLifecycleEventsListener / emitLifecycleEvent were added to public BugseeClient without updating @bugsee/webview's repeat-launch refusal list or inert client.
5. P2 — KILL_SDK turns the client off (enterKillState) with no Stopping/Stopped.

Residual / accepted (not blocking)

• BeforeReportUploaded firing inside assemble() before enqueue is a real sequencing lie, documented in the PR body.
• RelaunchedAfterCrash / blackout events are defined and not emitted, as stated.
• Trigger-queue overflow (maxQueueDepth) produces no lifecycle event; detection/upload() bypass the limiter. Folded into finding 3.
• First public await launch() cannot observe Launching/Launched (the listener is an instance method; platform launch() calls client.launch() before returning). Subsequent stop → launch cycles can. await launch() is the JS stand-in for the first Launched.
• setReportHandler(null) (or a replacement) between submit and settle drops the in-flight after — confirmed; snapshot at submit time as part of the P1 fix.

Positive

The live path is carefully done: Launching-before-providers / Launched-after, Stopping-before-drain / Stopped-after, veto skips both events and after, permanent vs retained classification matches isUploadSettled policy (permanent wins over retained), throwing sync observers are isolated, and the browser dialog order (BeforeShown → assemble/upload → AfterShown) is pinned end-to-end. Android wire names are right. after is finally invoked, which was the original hole (docs/review/core-B-capture-pipeline.md §11).

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

P1 High — async listener rejection is an unhandledRejection; onError never runs

emit only try/catches a synchronous throw. An async listener (or any callback that returns a rejected thenable) is the idiomatic JS form of “throwing”, and it bypasses this guard entirely.

Impact: The new public API documents “a throwing listener is routed to onError and never affects the SDK.” On Node this SDK already treats an unhandled rejection as a host process crash (never-throw.ts:27-31, Wave 2.5). A listener that does I/O and fails takes the app down; onError is silent.

Scenario (reproduced on this head):

client.setLifecycleEventsListener(async (type) => {
  if (type === LifecycleEvents.Launched) await fetch('/telemetry');
});
client.launch();

If fetch rejects, this emit returns normally, then one tick later the host sees unhandledRejection. Probe: onError=[], unhandled=['async listener boom'].

The same hole exists for ReportHandler.after (client.ts:742-746, also a raw try/catch). Vitest probe: logException with after: async () => { throw new Error('async after boom') } — onError not called, unhandledRejection fired.

Fix: Route both host callbacks through neverThrow (already exported from this package, already handles thenables). Add tests that assert an async throw is reported to onError and does not produce unhandledRejection — the existing tests only cover () => { throw ... }.

[cursor]

P1 High — same async hole as the lifecycle emitter

after is a host-facing callback whose contract is “a throwing handler goes to onError and never breaks reporting” (filters.ts:55-56). This try/catch only covers a sync throw. after: async () => { throw ... } or after: () => Promise.reject(...) schedules an unhandled rejection after logException has already resolved { ok: true }.

Reproduced with a vitest probe against this onSettled path: onError not called; unhandledRejection fired.

Also confirmed: this reads the live filters.report, so setReportHandler(null) (or a replacement) between submit and settle silently drops the in-flight after — including the one that paired with the before that already ran.

Fix: Snapshot after at submit time and run neverThrow(() => after?.(handled), onErrorSafe) (or equivalent).

[cursor]

P2 Medium — stop(); launch() emits Stopped after the new session is already live

launched = false is set synchronously, then Stopped is deferred to drainPending(...).then(...). drainPending always yields (Promise.allSettled then flush), even with nothing in flight. launch() only checks killed || launched, so a fire-and-forget stop followed by an immediate relaunch is allowed.

Impact: Apps that key cleanup / “SDK down” UI on LifecycleEvents.Stopped will tear down the new session. isLaunched() is true when Stopped arrives.

Scenario (reproduced):

client.launch();
void client.stop(); // forgot await — natural JS
client.launch();

Events: [Launching, Launched, Stopping, Launching, Launched, Stopped] with isLaunched() === true after Stopped.

Fix: Bind Stopped to the session that is stopping (generation / ignore a stale drain), or refuse launch() while a stop drain is in flight (T5 getStatus() Stopping). Also attach a .catch on this .then so a rejecting injectable flush cannot leave Stopping without Stopped.

Related: enterKillState (collector 99099) still sets launched = false with no Stopping/Stopped at all; a later stop() is a no-op (if (!launched) return true).

[cursor]

P2 Medium — upload/assembly events and after only fire on the live submitReport × assemble() path

onSettled is attached only here, and upload events are further gated on assembledReports, which is added only in the default assemble() closure (:532). Anything that delivers a report some other way is invisible to the new public observer.

Concrete paths that skip both the lifecycle events and after:

1. Durable recovery / next-launch retry. capture-recovery.ts marker rebuild and durable-upload-pipeline recover()/pump() call uploadPipeline.enqueue directly. A report that emitted ReportUploadFailedWithFutureRetry (or crashed mid-upload) can succeed on the next launch with no AfterReportUploaded. An app that keys a “crash pending” UI on these events never clears it.
2. Native-crash harvest (native-crash-recovery.ts:137) — same: enqueue only.
3. Custom trigger pipelines that never call assemble. @bugsee/webview (webview-report-pipeline.ts:46-62) and Electron renderer (renderer-report-pipeline.ts:48) always resolve {ok:true} (or {ok:false}) without going through assemble(). after does run (this onSettled still fires) but assembledReports.has is false, so a WebView/Electron-renderer listener never sees BeforeReportAssembled / AfterReportUploaded even when the incident was handed to native/main. Detected by the existing test “emits no upload events when an injected trigger pipeline never assembles” — that test pins the gap rather than a WebView/Electron event.
4. Trigger-queue overflow (trigger-pipeline.ts:79-81). report() resolves {ok:false} without calling assemble. Detection and upload() bypass the logException rate limiter, so a burst of unhandled rejections while one assemble is in-flight can drop reports with after running and no lifecycle event. The caller of detection is void submitReport(...), so the {ok:false} is discarded too.

Fix: Emit outcome events (and run after) from a single place that every delivery path shares — at minimum wrap durable enqueue / recover so a settled replay still produces AfterReportUploaded / ReportUploadFailed, and for WebView/Electron emit an honest handoff event (or AfterReportUploaded if you are willing to mean “handed off” rather than “HTTP PUT completed”). For overflow, emit ReportUploadFailed (or ReportAssemblyFailed) even when assembledReports is empty.

[cursor]

P2 Medium — emitLifecycleEvent is public BugseeClient API; WebView repeat-launch neither refuses it nor no-ops it

This is documented as a platform-tier hook (report dialog), but it is on the user-facing client every launch() returns — including @bugsee/webview, where export type Bugsee = BugseeClient.

restrictedClientFor (packages/webview/src/launch.ts:708-745) blocks setReportHandler / filters / stop / the container, then return { ...client, ...blocked }. It does not block setLifecycleEventsListener or emitLifecycleEvent, so a page-script BugseeWebView.launch('anything') keeps the real methods:

• setLifecycleEventsListener(fn) replaces the host bootstrap's listener (the gap the PR body already calls out).
• emitLifecycleEvent(AfterReportUploaded, { reportId: 'fake' }) injects a spoofed event into the host's still-installed listener without replacing it. That half is not mentioned in the PR.

The inert-client fallback (launch.ts:749-781) is worse: the new methods are simply missing. That file's own test comment says a missing method “is the TypeError this fallback exists to prevent.” inert.setLifecycleEventsListener(() => {}) throws in the host page.

Fix: Keep emitLifecycleEvent off the public client (pass it to the dialog as a closed-over function, as launch.ts already does). On WebView, add both methods to blocked (refuse) and to the inert surface (noop). Extend the refusal-list test so deleting them from blocked fails the way deleting setReportHandler already does.

[krassx]

Review findings triaged against current main (this PR branch is a frozen snapshot, so its diff does not show the fixes): P1 (async listener/after rejection) fixed in 37a8b06; handler snapshot at submit, rejecting-drain Stopped, JSDoc fixed in 7da7f3e; webview facade and KILL_SDK Stopped were already fixed on main; live-path-only upload events documented as a kept difference.

The overview PR #46 (head = main) was re-reviewed at ba608c4: no remaining findings, recommendation Approve.

🤖 Generated with Claude Code