diff --git a/src/commands/device.ts b/src/commands/device.ts index 099099e..f23eaaf 100644 --- a/src/commands/device.ts +++ b/src/commands/device.ts @@ -5,7 +5,12 @@ import { resolve as resolvePath } from 'node:path'; import { apiFlags } from '../config/flags/api.flags.js'; import { jsonFlag } from '../config/flags/output.flags.js'; -import { DeviceApiError, DeviceGateway } from '../gateways/device-gateway.js'; +import { + DeviceApiError, + DeviceGateway, + START_RECOVERY_WINDOW_MS, + startOutcomeUnknown, +} from '../gateways/device-gateway.js'; import type { AuthContext } from '../types/domain/auth.types.js'; import { type DeviceCatalogue, @@ -204,6 +209,39 @@ async function resolveApp( return { binaryUploadId }; } +/** + * A start that got no answer, or a proxy's 5xx in the API's place, may still + * have made a session, which is billed while it runs. Someone who just runs + * `dcd device start` again would have two. So name the sessions started in + * the last couple of minutes, and say to check before starting again. + */ +async function failedStartError(ctx: Ctx, error: unknown): Promise { + const reason = error instanceof Error ? error.message : String(error); + const status = error instanceof DeviceApiError ? error.status : 502; + const head = + 'Starting the session failed, but the request may still have started a session, and a session is billed while it runs.'; + // What to do first: a network error's message runs to a dozen lines. + const failed = (advice: string, session: DeviceSession | null = null) => + new DeviceApiError(`${head} ${advice}\n\nThe error: ${reason}`, status, null, session); + const minutes = START_RECOVERY_WINDOW_MS / 60_000; + const recent = await DeviceGateway.recentlyStarted(ctx.apiUrl, ctx.auth); + if (recent === null) { + return failed('Check for one with `dcd device list` before you start another.'); + } + + if (recent.length === 0) { + return failed(`No session was started in the last ${minutes} minutes, so it is safe to start one again.`); + } + + const listed = recent + .map((s) => `${s.id} (${s.client_label ?? 'no label'}, ${s.state}, started ${s.created_at})`) + .join(', '); + return failed( + `Open sessions started in the last ${minutes} minutes: ${listed}. If one is yours, use it instead of starting another; check it with \`dcd device status \`. Otherwise stop it with \`dcd device stop \`.`, + recent.length === 1 ? recent[0] : null, + ); +} + function parsePositiveInt(value: unknown, flag: string): number | undefined { if (value === undefined || value === '') return undefined; const n = Number(value); @@ -353,7 +391,14 @@ const startSub = defineCommand({ if (ttl) body.maxDurationMin = ttl; ctx.progress(ui.running(`Starting ${platform} device session…`)); - let { session } = await DeviceGateway.start(ctx.apiUrl, ctx.auth, body); + let session: DeviceSession; + try { + ({ session } = await DeviceGateway.start(ctx.apiUrl, ctx.auth, body)); + } catch (error) { + if (!startOutcomeUnknown(error)) throw error; + throw await failedStartError(ctx, error); + } + ctx.progress(ui.success(`Session ${session.id} started`)); // The session exists and bills from here on: every later failure must diff --git a/src/gateways/device-gateway.ts b/src/gateways/device-gateway.ts index 74b7277..e192396 100644 --- a/src/gateways/device-gateway.ts +++ b/src/gateways/device-gateway.ts @@ -24,6 +24,11 @@ import { ApiGateway } from './api-gateway.js'; */ export class DeviceApiError extends Error { code: null | string; + /** + * The API itself answered (its `{ statusCode, message }` envelope), not a + * proxy in front of it. A proxy's 502 can come after the API did the work. + */ + fromApi: boolean; session: DeviceSession | null; status: number; @@ -32,15 +37,31 @@ export class DeviceApiError extends Error { status: number, code: null | string, session: DeviceSession | null, + fromApi = false, ) { super(message); this.name = 'DeviceApiError'; this.status = status; this.code = code; this.session = session; + this.fromApi = fromApi; } } +/** How recent an open session must be to count as one a failed start may have made. */ +export const START_RECOVERY_WINDOW_MS = 2 * 60 * 1000; + +/** + * Whether a failed start may still have made a session: no answer arrived + * (the connection failed, or the reply was unreadable), or a proxy answered + * with a 5xx in the API's place. The API's own errors are refusals made + * before any session exists. + */ +export function startOutcomeUnknown(error: unknown): boolean { + if (!(error instanceof DeviceApiError)) return true; + return error.status >= 500 && !error.fromApi; +} + /** * The API's error envelope: `{ statusCode, message, error? }`, where `error` * is either Nest's status text or a machine code such as LIVE_NOT_FOUND. @@ -48,7 +69,7 @@ export class DeviceApiError extends Error { */ async function toDeviceApiError(res: Response, path: string): Promise { const text = await res.text(); - let body: { error?: unknown; message?: unknown; session?: unknown } = {}; + let body: { error?: unknown; message?: unknown; session?: unknown; statusCode?: unknown } = {}; try { body = JSON.parse(text); } catch { @@ -76,7 +97,10 @@ async function toDeviceApiError(res: Response, path: string): Promise( @@ -211,6 +235,23 @@ export const DeviceGateway = { return request(baseUrl, auth, 'GET', '/device/models'); }, + /** + * The open sessions started in the last START_RECOVERY_WINDOW_MS: the ones + * a start that got no answer may have made. Null when they can't be read. + */ + async recentlyStarted( + baseUrl: string, + auth: AuthContext, + now = Date.now(), + ): Promise { + try { + const { sessions } = await DeviceGateway.list(baseUrl, auth); + return sessions.filter((s) => now - Date.parse(s.created_at) <= START_RECOVERY_WINDOW_MS); + } catch { + return null; + } + }, + registerBundle( baseUrl: string, auth: AuthContext, diff --git a/src/mcp/bridge.ts b/src/mcp/bridge.ts index 3dab41e..91ee0f5 100644 --- a/src/mcp/bridge.ts +++ b/src/mcp/bridge.ts @@ -29,11 +29,10 @@ import { type Tool, } from '@modelcontextprotocol/sdk/types.js'; -import { DeviceGateway } from '../gateways/device-gateway.js'; +import { DeviceGateway, START_RECOVERY_WINDOW_MS } from '../gateways/device-gateway.js'; import { processDependencies, readTestYamlFileAsJson } from '../services/execution-plan.utils.js'; import { computeCommonRoot } from '../services/flow-paths.js'; import type { AuthContext } from '../types/domain/auth.types.js'; -import type { DeviceSession } from '../types/domain/device-session.types.js'; import { uploadLocalApp } from '../utils/app-upload.js'; import { getCliVersion } from '../utils/cli.js'; import { uploadFlowBundle } from '../utils/device-bundle.js'; @@ -165,9 +164,6 @@ export async function resolveLocalInputs( return args; } -/** How recent an open session must be to count as one a failed start may have made. */ -export const START_RECOVERY_WINDOW_MS = 2 * 60 * 1000; - /** * What to tell the agent when a start_device call failed without an answer * (the connection dropped, or the server never replied). The request may @@ -181,11 +177,8 @@ export async function explainFailedStart( now = Date.now(), ): Promise { const head = `start_device failed: ${message}. The request may still have started a session, and a session is billed while it runs.`; - let recent: DeviceSession[]; - try { - const { sessions } = await DeviceGateway.list(ctx.apiUrl, ctx.auth); - recent = sessions.filter((s) => now - Date.parse(s.created_at) <= START_RECOVERY_WINDOW_MS); - } catch { + const recent = await DeviceGateway.recentlyStarted(ctx.apiUrl, ctx.auth, now); + if (recent === null) { return `${head} Call list_devices to check for one before you call start_device again.`; } diff --git a/test/unit/device-command.test.ts b/test/unit/device-command.test.ts index a8fb123..c656f28 100644 --- a/test/unit/device-command.test.ts +++ b/test/unit/device-command.test.ts @@ -360,6 +360,88 @@ describe('dcd device', () => { }); }); + describe('a start that may have made a session anyway', () => { + // What Railway's proxy answers when the API did not reply in time. + const proxy502 = { + status: 502, + body: { status: 'error', code: 502, message: 'Application failed to respond', request_id: 'r-1' }, + }; + const justNow = () => new Date().toISOString(); + const tenMinutesAgo = () => new Date(Date.now() - 10 * 60_000).toISOString(); + + it('names the session a proxy 502 hid, and says how to stop it', async () => { + const calls = stubFetch((call) => + call.method === 'POST' + ? proxy502 + : { + body: { + sessions: [ + session('jolly-orange-lion', { state: 'provisioning', created_at: justNow() }), + session('old-one', { created_at: tenMinutesAgo() }), + ], + }, + }, + ); + const { exit, out } = await runJson('start', { platform: 'ios' }); + expect(exit).to.equal(DEVICE_EXIT.failed); + const message = (out.error as { message: string }).message; + expect(message) + .to.include('Application failed to respond') + .and.include('may still have started a session') + .and.include('jolly-orange-lion') + .and.include('dcd device stop'); + expect(message).to.not.include('old-one'); + expect((out.session as DeviceSession).id).to.equal('jolly-orange-lion'); + expect(calls.map((c) => c.method)).to.deep.equal(['POST', 'GET']); + }); + + it('says it is safe to start again when no session was made', async () => { + stubFetch((call) => + call.method === 'POST' + ? proxy502 + : { body: { sessions: [session('old-one', { created_at: tenMinutesAgo() })] } }, + ); + const { exit, out } = await runJson('start', { platform: 'ios' }); + expect(exit).to.equal(DEVICE_EXIT.failed); + expect((out.error as { message: string }).message).to.include('safe to start one again'); + expect(out.session ?? null).to.equal(null); + }); + + it('treats a start with no answer at all the same way', async () => { + stubFetch((call) => { + if (call.method === 'POST') throw new TypeError('fetch failed'); + return { body: { sessions: [session('jolly-orange-lion', { created_at: justNow() })] } }; + }); + const { exit, out } = await runJson('start', { platform: 'ios' }); + expect(exit).to.equal(DEVICE_EXIT.failed); + expect((out.error as { message: string }).message) + .to.include('may still have started a session') + .and.include('jolly-orange-lion'); + }); + + it('says to check `dcd device list` when the sessions cannot be read either', async () => { + stubFetch(() => proxy502); + const { exit, out } = await runJson('start', { platform: 'ios' }); + expect(exit).to.equal(DEVICE_EXIT.failed); + expect((out.error as { message: string }).message) + .to.include('may still have started a session') + .and.include('dcd device list'); + }); + + it("leaves the API's own errors alone: they come before any session exists", async () => { + for (const [status, message] of [ + [500, 'Failed to create live session'], + [503, 'Could not check your live spend cap right now (a temporary problem on our side). Please retry.'], + ] as const) { + const calls = stubFetch(() => ({ status, body: { statusCode: status, message, error: 'Error' } })); + const { exit, out } = await runJson('start', { platform: 'ios' }); + expect(exit).to.equal(DEVICE_EXIT.failed); + expect((out.error as { message: string }).message).to.equal(message); + expect(calls).to.have.length(1); + } + }); + }); + describe('start --model without --platform', () => { const catalogue = { ios: [{ model: 'iphone-16', name: 'iPhone 16', os: ['18'], default_os: '18', google_play: false, default: true }],