Stop unhandled rejection from Android logcat subprocess on teardown

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

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(),

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,

packages/platform-android/src/adb.ts

@@ -792,11 +792,13 @@ export const getLogcatTimestamp = async (adbId: string): Promise<string> => {
 
 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 = [

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<AppSessionState, { status: 'exited' }> =>
@@ -82,7 +74,10 @@ type CreateAndroidAppSessionOptions = {
   stopApp: () => Promise<void>;
   getAppPid: () => Promise<number | null>;
   getLogcatTimestamp: () => Promise<string>;
-  startLogcat: (args: readonly string[]) => Subprocess;
+  startLogcat: (
+    args: readonly string[],
+    options: { signal: AbortSignal }
+  ) => Subprocess;
   getDropboxOutput?: () => Promise<string>;
   getExitInfo?: () => Promise<string>;
   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]);
     },

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,