From 887376457c2f5c3f48893eb27570e36af2759fbf Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Tue, 14 Jul 2026 13:45:47 +0200 Subject: [PATCH] fix: stop unhandled rejection from Android logcat subprocess on teardown 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. --- .../version-plan-1784029535991.md | 5 ++ packages/jest/src/harness-session.ts | 8 ++- .../src/__tests__/instance.test.ts | 54 +++++++++++-------- packages/platform-android/src/adb.ts | 4 +- packages/platform-android/src/app-session.ts | 29 +++++----- packages/platform-android/src/instance.ts | 4 +- 6 files changed, 65 insertions(+), 39 deletions(-) create mode 100644 .nx/version-plans/version-plan-1784029535991.md diff --git a/.nx/version-plans/version-plan-1784029535991.md b/.nx/version-plans/version-plan-1784029535991.md new file mode 100644 index 00000000..e5287c7b --- /dev/null +++ b/.nx/version-plans/version-plan-1784029535991.md @@ -0,0 +1,5 @@ +--- +__default__: patch +--- + +Android E2E crash-detection logcat cleanup no longer surfaces an unhandled promise rejection during Harness shutdown, which previously crashed the whole test process. diff --git a/packages/jest/src/harness-session.ts b/packages/jest/src/harness-session.ts index f580aeee..356230bf 100644 --- a/packages/jest/src/harness-session.ts +++ b/packages/jest/src/harness-session.ts @@ -791,9 +791,13 @@ export const createHarnessSession = async ( let cleanupError: unknown; try { + // The app session (and its logcat/device log stream) must be fully + // torn down before the platform is disposed: platform disposal can + // shut down the device/emulator, and doing that concurrently with + // the app session's own cleanup races the device's log stream + // against the device disappearing out from under it. + await Promise.all([crashMonitor.dispose(), disposeCurrentAppSession()]); await Promise.all([ - crashMonitor.dispose(), - disposeCurrentAppSession(), bridge.dispose(), platformInstance.dispose(), metroInstance.dispose(), diff --git a/packages/platform-android/src/__tests__/instance.test.ts b/packages/platform-android/src/__tests__/instance.test.ts index 9ef26089..f5544d14 100644 --- a/packages/platform-android/src/__tests__/instance.test.ts +++ b/packages/platform-android/src/__tests__/instance.test.ts @@ -16,7 +16,9 @@ import * as emulatorStartup from '../emulator-startup.js'; const { getEmulatorCpuCores } = emulatorStartup; const createLogcatProcess = (lines: string[] = []): Subprocess => { - const process = { + // Mirrors nano-spawn's real Subprocess, which is both a Promise and an + // async iterable, since app-session.ts relies on it being thenable. + const process = Object.assign(Promise.resolve(undefined), { nodeChildProcess: Promise.resolve({ kill: vi.fn(), }), @@ -25,7 +27,7 @@ const createLogcatProcess = (lines: string[] = []): Subprocess => { yield line; } }, - }; + }); return process as unknown as Subprocess; }; @@ -545,16 +547,20 @@ describe('Android platform instance', () => { appSession.addListener(listener); await vi.advanceTimersByTimeAsync(0); - expect(startLogcat).toHaveBeenCalledWith('emulator-5554', [ - 'logcat', - '-v', - 'threadtime', - '-b', - 'crash', - '--uid=10234', - '-T', - '01-01 00:00:00.000', - ]); + expect(startLogcat).toHaveBeenCalledWith( + 'emulator-5554', + [ + 'logcat', + '-v', + 'threadtime', + '-b', + 'crash', + '--uid=10234', + '-T', + '01-01 00:00:00.000', + ], + { signal: expect.any(AbortSignal) } + ); expect(startLogcat.mock.invocationCallOrder[0]).toBeLessThan( startApp.mock.invocationCallOrder[0] ); @@ -679,16 +685,20 @@ describe('Android platform instance', () => { appSession.addListener(listener); await vi.advanceTimersByTimeAsync(0); - expect(startLogcat).toHaveBeenCalledWith('012345', [ - 'logcat', - '-v', - 'threadtime', - '-b', - 'crash', - '--uid=10234', - '-T', - '01-01 00:00:00.000', - ]); + expect(startLogcat).toHaveBeenCalledWith( + '012345', + [ + 'logcat', + '-v', + 'threadtime', + '-b', + 'crash', + '--uid=10234', + '-T', + '01-01 00:00:00.000', + ], + { signal: expect.any(AbortSignal) } + ); await expect(appSession.getState()).resolves.toEqual({ status: 'running', pid: 8765, diff --git a/packages/platform-android/src/adb.ts b/packages/platform-android/src/adb.ts index fde8644e..50e92506 100644 --- a/packages/platform-android/src/adb.ts +++ b/packages/platform-android/src/adb.ts @@ -792,11 +792,13 @@ export const getLogcatTimestamp = async (adbId: string): Promise => { export const startLogcat = ( adbId: string, - args: readonly string[] + args: readonly string[], + options?: { signal?: AbortSignal } ): Subprocess => spawn(getAdbBinaryPath(), ['-s', adbId, ...args], { stdout: 'pipe', stderr: 'pipe', + signal: options?.signal, }); export const DROPBOX_CRASH_TAGS = [ diff --git a/packages/platform-android/src/app-session.ts b/packages/platform-android/src/app-session.ts index 562f6f35..264865d8 100644 --- a/packages/platform-android/src/app-session.ts +++ b/packages/platform-android/src/app-session.ts @@ -62,14 +62,6 @@ const isCrashSignal = (line: string, bundleId: string): boolean => { ); }; -const stopSubprocess = async (child: Subprocess) => { - try { - (await child.nodeChildProcess).kill(); - } catch { - // Ignore termination failures for already-ended background processes. - } -}; - const isExitedState = ( state: AppSessionState ): state is Extract => @@ -82,7 +74,10 @@ type CreateAndroidAppSessionOptions = { stopApp: () => Promise; getAppPid: () => Promise; getLogcatTimestamp: () => Promise; - startLogcat: (args: readonly string[]) => Subprocess; + startLogcat: ( + args: readonly string[], + options: { signal: AbortSignal } + ) => Subprocess; getDropboxOutput?: () => Promise; getExitInfo?: () => Promise; crashArtifactWriter?: CrashArtifactWriter; @@ -193,7 +188,17 @@ export const createAndroidAppSession = async ({ const logcatTimestamp = await getLogcatTimestamp(); const sessionStartedAt = Date.now(); - const logcatProcess = startLogcat(getLogcatArgs(appUid, logcatTimestamp)); + const logcatAbortController = new AbortController(); + const logcatProcess = startLogcat(getLogcatArgs(appUid, logcatTimestamp), { + signal: logcatAbortController.signal, + }); + // Aborting is nano-spawn's own cancellation path, so the resulting + // SubprocessError is settled the same way regardless of which of the + // merged stdout/stderr iterators observes it first. Without this, killing + // the underlying process directly can make both iterators reject at once, + // and nano-spawn's internal `Promise.race` only reports one of them, + // leaving the other an unhandled rejection. + logcatProcess.catch(() => undefined); const crashReporter = createAndroidCrashReporter({ bundleId, crashArtifactWriter, @@ -239,7 +244,7 @@ export const createAndroidAppSession = async ({ disposed = true; stopPolling = true; emitter.clear(); - await stopSubprocess(logcatProcess); + logcatAbortController.abort(); await Promise.allSettled([logTask]); throw error; } @@ -287,7 +292,7 @@ export const createAndroidAppSession = async ({ cancelPendingPollDelay(); emitter.clear(); - await stopSubprocess(logcatProcess); + logcatAbortController.abort(); await stopApp(); await Promise.allSettled([logTask, pollTask]); }, diff --git a/packages/platform-android/src/instance.ts b/packages/platform-android/src/instance.ts index 9e2fa362..67ce7051 100644 --- a/packages/platform-android/src/instance.ts +++ b/packages/platform-android/src/instance.ts @@ -297,7 +297,7 @@ export const getAndroidEmulatorPlatformInstance = async ( stopApp: () => adb.stopApp(adbId, config.bundleId), getAppPid: () => adb.getAppPid(adbId, config.bundleId), getLogcatTimestamp: () => adb.getLogcatTimestamp(adbId), - startLogcat: (args) => adb.startLogcat(adbId, args), + startLogcat: (args, options) => adb.startLogcat(adbId, args, options), getDropboxOutput: () => adb.getDropboxPrint(adbId), getExitInfo: () => adb.getActivityExitInfo(adbId, config.bundleId), crashArtifactWriter: init.crashArtifactWriter, @@ -370,7 +370,7 @@ export const getAndroidPhysicalDevicePlatformInstance = async ( stopApp: () => adb.stopApp(adbId, config.bundleId), getAppPid: () => adb.getAppPid(adbId, config.bundleId), getLogcatTimestamp: () => adb.getLogcatTimestamp(adbId), - startLogcat: (args) => adb.startLogcat(adbId, args), + startLogcat: (args, options) => adb.startLogcat(adbId, args, options), getDropboxOutput: () => adb.getDropboxPrint(adbId), getExitInfo: () => adb.getActivityExitInfo(adbId, config.bundleId), crashArtifactWriter: init?.crashArtifactWriter,