Stop unhandled rejection from Android logcat subprocess on teardown - #162
Merged
Conversation
Killing the adb logcat child process directly could make nano-spawn's merged stdout/stderr iterators both reject at once; only one rejection was ever observed by the try/catch around the for-await loop, leaving the other unhandled and crashing the whole E2E job. Use nano-spawn's own AbortSignal-based cancellation instead, and stop racing app-session teardown against emulator shutdown so the device's log stream is fully closed before the device itself goes away.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
V3RON
changed the title
V3RON
added a commit
that referenced
this pull request
Jul 20, 2026
…#164) ## What is this? Removes a redundant, competing subprocess-shutdown mechanism so that Harness has exactly one authority for tearing down child processes (emulators, simulators, logcat, the iOS XCTest agent, browsers) when a test session ends or is interrupted. ## How does it work? The jest harness session already owned a structured, ordered `dispose()` driven by an `AbortController`. Separately, `packages/tools/src/spawn.ts` installed its own global `SIGINT`/`SIGTERM` handlers that raw-killed every tracked child process in parallel with that graceful dispose — the same class of race a prior fix (#162) had to patch for Android logcat specifically, just not everywhere else. This change removes that global net entirely and makes the session's single `AbortController` (`sessionController`) the only signal handlers in the process, for its whole lifetime — early SIGINT/SIGTERM during setup, and `dispose()`'s teardown afterward. `HarnessPlatformInitOptions.signal` is now documented and used as session-lifetime (not scoped to platform init), and is threaded into every platform runner's long-lived children: - Android: combined with the app session's own logcat abort controller via `AbortSignal.any`. - iOS: aborts the `--console` launch process and stops the XCTest agent. - Web: closes the Playwright browser. - Vega: disposes the current app session (stopping its poll loop and the app). A platform readiness timeout no longer forges an abort reason onto the session signal — it races independently and, on timeout, the session's own abort cancels the abandoned init call. The SIGTERM → wait → SIGKILL tail that used to live only inside iOS's XCTest agent code is now a shared `terminate()` helper in `packages/tools`, reused by both the agent and the app-session launch process. ## Why is this useful? Two independent teardown paths racing on every signal was a source of exactly the bug class #162 already had to fix once — this removes the redundant path instead of patching it platform-by-platform. It also makes the abort contract explicit and consistent across all four platform runners, so a new runner only has to observe one signal correctly instead of guessing whether a second, implicit cleanup mechanism might also be reaching in.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is this?
Fixes a crash in the Android E2E CI job where Harness shutdown could throw an unhandled
SubprocessErrorfrom theadb logcatprocess (Command failed with exit code 255: adb ... logcat -v threadtime -b crash ...), killing the whole test process even though the app session's own teardown code already wraps that stream in a try/catch.How does it work?
The Android app session's logcat stream pipes both stdout and stderr, and
nano-spawnmerges them into a single iterable using an internalPromise.race. Killing the underlying process directly (childProcess.kill()) could make both the stdout and stderr sides observe the process death and reject at nearly the same time; only the "winning" rejection was ever seen by the surrounding try/catch, leaving the other one unhandled.Two changes fix this:
AbortController/AbortSignalpassed intonano-spawn's ownsignaloption, which is its supported cancellation path, instead of killing the child process handle directly.Promise.all, so the emulator could be torn down whileadb logcatwas still attached to it, which is what produced the adb-side exit code 255 in the first place.Why is this useful?
The Android E2E and crash-validation CI jobs no longer intermittently fail with an unrelated, misleading
SubprocessErrorduring normal shutdown.