diff --git a/CHANGELOG.md b/CHANGELOG.md index b7fe16ff05..c970d81f5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ This is the log of notable changes to EAS CLI and related packages. ### ๐Ÿ› Bug fixes +- [build-tools] Stop agent-device artifact polling when remote simulator sessions end. ([#4115](https://github.com/expo/eas-cli/pull/4115) by [@szdziedzic](https://github.com/szdziedzic)) - [eas-cli] clean up error handling in local builds. ([#4105](https://github.com/expo/eas-cli/pull/4105) by [@douglowder](https://github.com/douglowder)) ### ๐Ÿงน Chores diff --git a/packages/build-tools/src/steps/functions/__tests__/startAgentDeviceRemoteSession.test.ts b/packages/build-tools/src/steps/functions/__tests__/startAgentDeviceRemoteSession.test.ts index 5d6f688c58..8e81bbb3ad 100644 --- a/packages/build-tools/src/steps/functions/__tests__/startAgentDeviceRemoteSession.test.ts +++ b/packages/build-tools/src/steps/functions/__tests__/startAgentDeviceRemoteSession.test.ts @@ -1,7 +1,10 @@ import { type bunyan } from '@expo/logger'; import { Sentry } from '../../../sentry'; -import { stopAgentDeviceEventCollectionSafelyAsync } from '../startAgentDeviceRemoteSession'; +import { + stopAgentDeviceArtifactPollingAndDaemonAsync, + stopAgentDeviceEventCollectionSafelyAsync, +} from '../startAgentDeviceRemoteSession'; jest.mock('../../../sentry'); @@ -37,3 +40,73 @@ describe(stopAgentDeviceEventCollectionSafelyAsync, () => { ); }); }); + +describe(stopAgentDeviceArtifactPollingAndDaemonAsync, () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('aborts and awaits artifact polling before stopping the daemon', async () => { + const order: string[] = []; + const abortController = new AbortController(); + let resolvePolling!: () => void; + const artifactPollingPromise = new Promise(resolve => { + resolvePolling = () => { + order.push('polling'); + resolve(); + }; + }); + const daemonProcess = { + stopAsync: jest.fn(async () => { + order.push('daemon'); + }), + }; + + const stoppingPromise = stopAgentDeviceArtifactPollingAndDaemonAsync({ + artifactPollAbortController: abortController, + artifactPollingPromise, + daemonProcess, + deviceRunSessionId: 'session-id', + logger: { warn: jest.fn() } as unknown as bunyan, + }); + + expect(abortController.signal.aborted).toBe(true); + expect(daemonProcess.stopAsync).not.toHaveBeenCalled(); + + resolvePolling(); + await stoppingPromise; + + expect(order).toEqual(['polling', 'daemon']); + }); + + it('reports a polling failure and still stops the daemon', async () => { + const error = new Error('polling failed'); + const logger = { warn: jest.fn() } as unknown as bunyan; + const daemonProcess = { stopAsync: jest.fn().mockResolvedValue(undefined) }; + + await expect( + stopAgentDeviceArtifactPollingAndDaemonAsync({ + artifactPollAbortController: new AbortController(), + artifactPollingPromise: Promise.reject(error), + daemonProcess, + deviceRunSessionId: 'session-id', + logger, + }) + ).resolves.toBeUndefined(); + + expect(Sentry.capture).toHaveBeenCalledWith( + 'Could not finish agent-device remote session artifact polling', + error, + { + level: 'warning', + tags: { phase: 'agent-device-artifact-polling', operation: 'stop' }, + extras: { deviceRunSessionId: 'session-id' }, + } + ); + expect(logger.warn).toHaveBeenCalledWith( + { err: error }, + 'Could not finish agent-device remote session artifact polling.' + ); + expect(daemonProcess.stopAsync).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/build-tools/src/steps/functions/startAgentDeviceRemoteSession.ts b/packages/build-tools/src/steps/functions/startAgentDeviceRemoteSession.ts index e73990886f..87c2b27849 100644 --- a/packages/build-tools/src/steps/functions/startAgentDeviceRemoteSession.ts +++ b/packages/build-tools/src/steps/functions/startAgentDeviceRemoteSession.ts @@ -98,6 +98,11 @@ export function createStartAgentDeviceRemoteSessionBuildFunction( let eventCollection: | Awaited> | undefined; + const artifactPollAbortController = new AbortController(); + const artifactPollSignal = signal + ? AbortSignal.any([signal, artifactPollAbortController.signal]) + : artifactPollAbortController.signal; + let artifactPollingPromise: Promise | undefined; try { // serve-sim is iOS-only โ€” only launch it (and report a webPreviewUrl) // on Darwin. Android sessions go without a preview URL. @@ -121,11 +126,12 @@ export function createStartAgentDeviceRemoteSessionBuildFunction( }, logger, }); - void pollAgentDeviceArtifactsForUploadAsync(ctx, { + artifactPollingPromise = pollAgentDeviceArtifactsForUploadAsync(ctx, { deviceRunSessionId, daemonUrl: `http://127.0.0.1:${daemonPort}`, daemonToken, logger, + signal: artifactPollSignal, }); eventCollection = await startAgentDeviceEventCollectionAsync({ @@ -142,23 +148,62 @@ export function createStartAgentDeviceRemoteSessionBuildFunction( signal, }); } finally { - if (serveSim) { - await serveSim.stopAsync(); - } - await agentDeviceTunnel.stopAsync(); - if (eventCollection) { - await stopAgentDeviceEventCollectionSafelyAsync({ - eventCollection, + try { + if (serveSim) { + await serveSim.stopAsync(); + } + await agentDeviceTunnel.stopAsync(); + if (eventCollection) { + await stopAgentDeviceEventCollectionSafelyAsync({ + eventCollection, + deviceRunSessionId, + logger, + }); + } + } finally { + await stopAgentDeviceArtifactPollingAndDaemonAsync({ + artifactPollAbortController, + artifactPollingPromise, + daemonProcess, deviceRunSessionId, logger, }); } - await daemonProcess.stopAsync(); } }, }); } +export async function stopAgentDeviceArtifactPollingAndDaemonAsync({ + artifactPollAbortController, + artifactPollingPromise, + daemonProcess, + deviceRunSessionId, + logger, +}: { + artifactPollAbortController: AbortController; + artifactPollingPromise: Promise | undefined; + daemonProcess: Pick; + deviceRunSessionId: string; + logger: bunyan; +}): Promise { + artifactPollAbortController.abort(); + if (artifactPollingPromise) { + try { + await artifactPollingPromise; + } catch (err) { + const error = err instanceof Error ? err : new Error(String(err)); + Sentry.capture('Could not finish agent-device remote session artifact polling', error, { + level: 'warning', + tags: { phase: 'agent-device-artifact-polling', operation: 'stop' }, + extras: { deviceRunSessionId }, + }); + logger.warn({ err: error }, 'Could not finish agent-device remote session artifact polling.'); + } + } + await daemonProcess.stopAsync(); +} + export async function stopAgentDeviceEventCollectionSafelyAsync({ eventCollection, deviceRunSessionId, diff --git a/packages/build-tools/src/steps/utils/__tests__/agentDeviceArtifacts.test.ts b/packages/build-tools/src/steps/utils/__tests__/agentDeviceArtifacts.test.ts index 94abed713f..986464da00 100644 --- a/packages/build-tools/src/steps/utils/__tests__/agentDeviceArtifacts.test.ts +++ b/packages/build-tools/src/steps/utils/__tests__/agentDeviceArtifacts.test.ts @@ -14,6 +14,32 @@ import { jest.mock('../deviceRunSessionArtifacts'); jest.mock('../../../sentry'); jest.mock('node-fetch'); +jest.mock('node:timers/promises', () => { + const actual = jest.requireActual('node:timers/promises'); + return { + ...actual, + setTimeout: jest.fn( + (delay: number, value: unknown, options?: { signal?: AbortSignal }) => + new Promise((resolve, reject) => { + const { signal } = options ?? {}; + if (signal?.aborted) { + reject(signal.reason); + return; + } + + const onAbort = (): void => { + clearTimeout(timeout); + reject(signal?.reason); + }; + const timeout = setTimeout(() => { + signal?.removeEventListener('abort', onAbort); + resolve(value); + }, delay); + signal?.addEventListener('abort', onAbort, { once: true }); + }) + ), + }; +}); const { Response } = jest.requireActual('node-fetch') as typeof import('node-fetch'); @@ -91,6 +117,7 @@ describe(listAgentDeviceArtifactsAsync, () => { ]); expect(jest.mocked(fetch)).toHaveBeenCalledWith('http://127.0.0.1:1234/artifacts', { headers: { Authorization: 'Bearer daemon-token' }, + signal: undefined, }); }); @@ -199,15 +226,17 @@ describe(pollAgentDeviceArtifactsForUploadAsync, () => { it('reports artifact listing errors on every fifth consecutive failure', async () => { const logger = createLoggerMock(); const ctx = {} as unknown as CustomBuildContext; + const abortController = new AbortController(); const error = new Error('daemon is not ready'); jest.mocked(fetch).mockRejectedValue(error); - void pollAgentDeviceArtifactsForUploadAsync(ctx, { + const pollingPromise = pollAgentDeviceArtifactsForUploadAsync(ctx, { deviceRunSessionId: 'drs-id', daemonUrl: 'http://127.0.0.1:1234', daemonToken: 'daemon-token', logger, + signal: abortController.signal, }); await flushPromisesAsync(); @@ -234,12 +263,16 @@ describe(pollAgentDeviceArtifactsForUploadAsync, () => { 'Could not list agent-device remote session artifacts.' ); expect(Sentry.capture).toHaveBeenCalledTimes(2); + + abortController.abort(); + await pollingPromise; }); it('retries an artifact after a failed upload', async () => { const data = Buffer.from('artifact-data'); const logger = createLoggerMock(); const ctx = {} as unknown as CustomBuildContext; + const abortController = new AbortController(); const artifact = { id: 'artifact-id', filename: 'report.json', @@ -259,11 +292,12 @@ describe(pollAgentDeviceArtifactsForUploadAsync, () => { await readStreamAsync(stream); }); - void pollAgentDeviceArtifactsForUploadAsync(ctx, { + const pollingPromise = pollAgentDeviceArtifactsForUploadAsync(ctx, { deviceRunSessionId: 'drs-id', daemonUrl: 'http://127.0.0.1:1234', daemonToken: 'daemon-token', logger, + signal: abortController.signal, }); await flushPromisesAsync(); @@ -273,11 +307,44 @@ describe(pollAgentDeviceArtifactsForUploadAsync, () => { await flushPromisesAsync(); expect(jest.mocked(uploadDeviceRunSessionArtifactAsync)).toHaveBeenCalledTimes(2); + + abortController.abort(); + await pollingPromise; + }); + + it('stops polling promptly when aborted', async () => { + const logger = createLoggerMock(); + const ctx = {} as unknown as CustomBuildContext; + const abortController = new AbortController(); + + jest.mocked(fetch).mockResolvedValue(new Response(JSON.stringify({ artifacts: [] }))); + + const pollingPromise = pollAgentDeviceArtifactsForUploadAsync(ctx, { + deviceRunSessionId: 'drs-id', + daemonUrl: 'http://127.0.0.1:1234', + daemonToken: 'daemon-token', + logger, + signal: abortController.signal, + }); + + await flushPromisesAsync(); + expect(jest.mocked(fetch)).toHaveBeenCalledTimes(1); + expect(jest.mocked(fetch)).toHaveBeenCalledWith('http://127.0.0.1:1234/artifacts', { + headers: { Authorization: 'Bearer daemon-token' }, + signal: abortController.signal, + }); + + abortController.abort(); + await pollingPromise; + + await jest.advanceTimersByTimeAsync(10_000); + expect(jest.mocked(fetch)).toHaveBeenCalledTimes(1); }); it('stops polling when the daemon does not expose artifact inventory', async () => { const logger = createLoggerMock(); const ctx = {} as unknown as CustomBuildContext; + const abortController = new AbortController(); jest.mocked(fetch).mockResolvedValueOnce(new Response('Not found', { status: 404 })); @@ -287,6 +354,7 @@ describe(pollAgentDeviceArtifactsForUploadAsync, () => { daemonUrl: 'http://127.0.0.1:1234', daemonToken: 'daemon-token', logger, + signal: abortController.signal, }) ).resolves.toBeUndefined(); diff --git a/packages/build-tools/src/steps/utils/agentDeviceArtifacts.ts b/packages/build-tools/src/steps/utils/agentDeviceArtifacts.ts index 873dd94f9c..3c1488270e 100644 --- a/packages/build-tools/src/steps/utils/agentDeviceArtifacts.ts +++ b/packages/build-tools/src/steps/utils/agentDeviceArtifacts.ts @@ -6,12 +6,12 @@ import fetch from 'node-fetch'; import os from 'node:os'; import path from 'node:path'; import { pipeline } from 'node:stream/promises'; +import { setTimeout as setTimeoutAsync } from 'node:timers/promises'; import { z } from 'zod'; import { CustomBuildContext } from '../../customBuildContext'; import { Sentry } from '../../sentry'; import { formatBytes } from '../../utils/artifacts'; -import { sleepAsync } from '../../utils/retry'; import { uploadDeviceRunSessionArtifactAsync } from './deviceRunSessionArtifacts'; const AGENT_DEVICE_ARTIFACT_UPLOAD_POLL_INTERVAL_MS = 5_000; @@ -40,20 +40,22 @@ export async function pollAgentDeviceArtifactsForUploadAsync( daemonUrl, daemonToken, logger, + signal, }: { deviceRunSessionId: string; daemonUrl: string; daemonToken: string; logger: bunyan; + signal: AbortSignal; } ): Promise { logger.info('Started polling agent-device daemon for artifacts.'); const uploadedArtifactIds = new Set(); let listArtifactsErrorCount = 0; - for (;;) { + while (!signal.aborted) { try { - const artifacts = await listAgentDeviceArtifactsAsync({ daemonUrl, daemonToken }); + const artifacts = await listAgentDeviceArtifactsAsync({ daemonUrl, daemonToken, signal }); listArtifactsErrorCount = 0; for (const artifact of artifacts) { if (uploadedArtifactIds.has(artifact.id)) { @@ -75,6 +77,9 @@ export async function pollAgentDeviceArtifactsForUploadAsync( } } } catch (err) { + if (signal.aborted) { + break; + } if (err instanceof AgentDeviceArtifactsUnsupportedError) { logger.warn( 'agent-device daemon does not expose artifact inventory; remote session artifact uploads are disabled.' @@ -91,19 +96,24 @@ export async function pollAgentDeviceArtifactsForUploadAsync( ); } } - await sleepAsync(AGENT_DEVICE_ARTIFACT_UPLOAD_POLL_INTERVAL_MS); + await sleepUntilAbortedAsync(AGENT_DEVICE_ARTIFACT_UPLOAD_POLL_INTERVAL_MS, signal); } + + logger.info('Agent-device artifact polling stopped.'); } export async function listAgentDeviceArtifactsAsync({ daemonUrl, daemonToken, + signal, }: { daemonUrl: string; daemonToken: string; + signal?: AbortSignal; }): Promise { const response = await fetch(new URL('/artifacts', daemonUrl).toString(), { headers: { Authorization: `Bearer ${daemonToken}` }, + signal, }); if (response.status === 404) { throw new AgentDeviceArtifactsUnsupportedError(); @@ -191,3 +201,13 @@ async function downloadAgentDeviceArtifactToFileAsync({ } await pipeline(response.body, createWriteStream(destinationPath)); } + +async function sleepUntilAbortedAsync(timeoutMs: number, signal: AbortSignal): Promise { + try { + await setTimeoutAsync(timeoutMs, undefined, { signal }); + } catch (err) { + if (!signal.aborted) { + throw err; + } + } +}