fix(android): handle missing adb reverse mapping in reverseRemove#4927
Open
duckdum wants to merge 1 commit intowix:masterfrom
Open
fix(android): handle missing adb reverse mapping in reverseRemove#4927duckdum wants to merge 1 commit intowix:masterfrom
duckdum wants to merge 1 commit intowix:masterfrom
Conversation
When the instrumentation process terminates unexpectedly (e.g., due to ADB instability with multiple emulators sharing one ADB server), the close event callback calls reverseRemove() to clean up the port mapping. If the mapping was already removed (by the same ADB hiccup that killed the instrumentation), reverseRemove() throws "listener not found". Since the close event callback is async but never awaited, this becomes an unhandled promise rejection. Jest-circus captures unhandled rejections and attributes them to whichever test is currently running, causing that test to fail even though the error is unrelated to the test logic. The emulator itself recovers fine on the next device.launchApp() call. This change catches the "not found" error in reverseRemove() since removing an already-absent mapping is a benign no-op. Other errors are still thrown.
5 tasks
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.
Description
This pull request addresses an issue with parallel Android emulator test execution where
ADB.reverseRemove()throws an unhandled error that causes random test failures.Problem
When running Detox tests with multiple Android emulators (
--maxWorkers 2+), tests randomly fail with:followed by:
Root Cause
With multiple emulators sharing a single ADB server, the server occasionally drops the
adb reverseport mapping on one emulator. When this happens:closeevent on the child process fires, triggeringInstrumentation._onTerminated()→MonitoredInstrumentation._onInstrumentationTerminated()→ theAndroidDrivertermination functionADB.reverseRemove(), which triesadb reverse --remove tcp:<port>— but the mapping is already gone, so it throws"listener not found"closeevent callback isasyncbut neverawaited, this becomes an unhandled promise rejectionprocess.on('unhandledRejection')and attributes them to whichever test is currently running — causing that test to fail even though the error is unrelated to test logicThe emulator itself recovers fine on the next
device.launchApp()call (which re-creates the reverse mapping). The only damage is the collateral test failure from the unhandled rejection.Fix
Catch the
"not found"error inADB.reverseRemove()since removing an already-absent mapping is a benign no-op. All other errors are still thrown.This is consistent with how other ADB methods in this file handle expected failure cases (e.g.,
getState()catching "device not found",grantAllPermissions()catching "no permission specified",getFileSize()handling "No such file or directory").Relation to #4900
PR #4900 addresses the broader ADB instability by giving each emulator its own ADB server. This PR is complementary — it provides resilience at the
reverseRemovelevel regardless of the ADB server architecture. Even with per-emulator ADB servers, a mapping could still be absent during cleanup if the emulator crashed, so this defensive check is valuable in either case.