From 27a023eb44d408f1ce8267e16644bf8fc7b1c9b8 Mon Sep 17 00:00:00 2001 From: finalerock44 Date: Wed, 4 Nov 2026 12:53:14 +0000 Subject: [PATCH] fix(device): a start that got no answer says a session may still have started `dcd device start` printed a bare error when POST /device/sessions got no answer (a network error) or a proxy's 502 in the API's place. The API may have created the session anyway, and it is billed while it runs, so running the command again could leave two. When the outcome is unknown, the error now says so first and lists the open sessions started in the last 2 minutes (the one in the --json envelope's session when there is exactly one), with how to check or stop them, or says it is safe to start again. The API's own errors (its { statusCode, message } envelope) still come through unchanged: those are refusals made before any session exists. The recent-session lookup moves into DeviceGateway.recentlyStarted, shared with the MCP bridge's explanation of a failed start_device. --- src/commands/device.ts | 49 ++++++++++++++++++- src/gateways/device-gateway.ts | 45 +++++++++++++++++- src/mcp/bridge.ts | 13 ++--- test/unit/device-command.test.ts | 82 ++++++++++++++++++++++++++++++++ 4 files changed, 175 insertions(+), 14 deletions(-) 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 }],