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.
|
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
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.