Skip to content

fix(performance): close CPU, FPS and log collectors when their start is cancelled - #66

Merged
lauridskern merged 1 commit into
mainfrom
fix/collector-startup-cancellation
Oct 29, 2026
Merged

lauridskern merged 1 commit into
mainfrom
fix/collector-startup-cancellation

Conversation

@lauridskern

@lauridskern lauridskern commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

mobile_cpu_session, mobile_display_fps_session and mobile_logs_session opened their collector after device validation without checking for cancellation. A cancel during validation still started the collector, but the client never received its sessionId. For CPU, the per-app lock then blocked retries until the five-minute idle expiry.

These starts now open through the existing openRequestSession helper, which the simulator and mirror streams already use. A cancelled start opens nothing, a session that opens after cancellation or loses its response is closed, and a delivered session keeps its normal lifetime.

Telemetry

The safe wrappers pass the request signal to captureServerError, so failures after cancellation count as cancelled. Cleanup failures use the existing stream.cancel_cleanup capture, noted in docs/telemetry.md. Trace propagation and operation names are unchanged.

Validation

  • packages/core/test/collector-startup.test.ts cancels each start during validation, checks that no collector is left running, then checks that a delivered session stays readable. All three tests fail on the base commit and pass with the fix.
  • Focused node --test (new file plus cpu-tools, cpu-sessions, logs, physical-ios-logs, performance-recordings, telemetry): 67/67 pass.
  • npm test: 897 pass, 1 skipped, 1 fail (existing test needing the absent Baguette binary).

…is cancelled

Session starts now open through openRequestSession. A request cancelled during
device validation opens nothing, a session that opens after cancellation or
before an undelivered response is closed, and a delivered session keeps its
normal lifetime.
@lauridskern
lauridskern marked this pull request as ready for review October 29, 2026 14:59
@lauridskern
lauridskern merged commit ab331a6 into main Oct 29, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant