Skip to content

Commit 5f3aa74

Browse files
sorccuclaude
andcommitted
feat(cli): fail fast when async test-session dispatch fails [RED-680]
checkly test now runs sessions via POST /v1/test-sessions/run, which dispatches the check runs in the background after the 202. The CLI watches the returned scheduling operation (long-poll with 408 retries, aborted once the run settles) and fails the run immediately with the server's error when dispatch fails — previously a server-side dispatch failure produced no MQTT results and the CLI hung for the full per-check timeout (600s, or 1200s for Playwright) before printing a generic timeout. In --detach mode the CLI confirms dispatch reached a terminal state before detaching, so a dispatch failure still exits non-zero like the synchronous endpoint did. Requires the backend scheduling endpoints (checkly/monorepo#2795) — do not release before they are live in production. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HFcrNm7uzzkD1p87Nf9AhP
1 parent 82d702e commit 5f3aa74

5 files changed

Lines changed: 356 additions & 8 deletions

File tree

packages/cli/src/rest/__tests__/test-sessions.spec.ts

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ describe('TestSessions REST client', () => {
4141
shouldRecord: true,
4242
} as any)
4343

44-
expect(api.post).toHaveBeenCalledWith('/next/test-sessions/run', expect.objectContaining({
44+
expect(api.post).toHaveBeenCalledWith('/v1/test-sessions/run', expect.objectContaining({
4545
repoInfo: githubRepoInfo,
4646
}), expect.any(Object))
4747
})
@@ -126,6 +126,71 @@ describe('TestSessions REST client', () => {
126126
})
127127
})
128128

129+
it('runs a test session via the async v1 endpoint', async () => {
130+
const api = {
131+
post: vi.fn().mockResolvedValue({ data: { schedulingId: 'op-id', sequenceIds: {} } }),
132+
}
133+
const testSessions = new TestSessions(api as any)
134+
135+
const payload = { name: 'project', checkRunJobs: [], project: { logicalId: 'p' }, runLocation: 'eu-central-1', shouldRecord: false }
136+
await testSessions.run(payload as any)
137+
138+
expect(api.post).toHaveBeenCalledWith('/v1/test-sessions/run', payload, expect.objectContaining({
139+
transformRequest: expect.any(Function),
140+
}))
141+
})
142+
143+
it('gets scheduling completion by ID', async () => {
144+
const api = {
145+
get: vi.fn().mockResolvedValue({ data: { schedulingId: 'op-id', status: 'SUCCEEDED', error: null } }),
146+
}
147+
const testSessions = new TestSessions(api as any)
148+
const signal = new AbortController().signal
149+
150+
await testSessions.getSchedulingCompletion('op-id', { signal })
151+
152+
expect(api.get).toHaveBeenCalledWith('/v1/test-sessions/scheduling/op-id/completion', {
153+
params: { maxWaitSeconds: 30 },
154+
signal,
155+
})
156+
})
157+
158+
it('polls scheduling completion timeouts until complete', async () => {
159+
const completed = { schedulingId: 'op-id', status: 'FAILED', error: { code: 'SCHEDULING_ERROR', message: 'nope' } }
160+
const api = {
161+
get: vi.fn()
162+
.mockRejectedValueOnce(new RequestTimeoutError({
163+
statusCode: 408,
164+
error: 'Request Timeout',
165+
message: 'Scheduling is still in progress, but maximum per-request wait time was exceeded. Please retry.',
166+
}))
167+
.mockResolvedValueOnce({ data: completed }),
168+
}
169+
const testSessions = new TestSessions(api as any)
170+
171+
const result = await testSessions.pollSchedulingUntilComplete('op-id')
172+
173+
expect(result).toEqual(completed)
174+
expect(api.get).toHaveBeenCalledTimes(2)
175+
})
176+
177+
it('stops polling scheduling when the signal is aborted', async () => {
178+
const api = {
179+
get: vi.fn().mockRejectedValue(new RequestTimeoutError({
180+
statusCode: 408,
181+
error: 'Request Timeout',
182+
message: 'Still in progress.',
183+
})),
184+
}
185+
const testSessions = new TestSessions(api as any)
186+
const controller = new AbortController()
187+
controller.abort()
188+
189+
await expect(testSessions.pollSchedulingUntilComplete('op-id', { signal: controller.signal }))
190+
.rejects.toBeInstanceOf(RequestTimeoutError)
191+
expect(api.get).toHaveBeenCalledTimes(1)
192+
})
193+
129194
it('lists public test sessions with filters', async () => {
130195
const api = {
131196
get: vi.fn().mockResolvedValue({ data: { length: 0, entries: [], nextId: null } }),

packages/cli/src/rest/test-sessions.ts

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,45 @@ export type TriggerTestSessionResponse = {
4949
sequenceIds: Record<string, SequenceId>
5050
}
5151

52+
export type RunTestSessionResponse = {
53+
/** The scheduling operation dispatching this run's check runs in the background. */
54+
schedulingId: string
55+
/** Only present for recorded runs (shouldRecord: true). */
56+
testSessionId?: string
57+
testResultIds?: Record<string, string>
58+
sequenceIds: Record<string, SequenceId>
59+
}
60+
61+
export type TestSessionSchedulingStatus = 'PENDING' | 'RUNNING' | 'SUCCEEDED' | 'FAILED'
62+
63+
export type TestSessionSchedulingOperation = {
64+
schedulingId: string
65+
testSessionId: string | null
66+
status: TestSessionSchedulingStatus
67+
checksTotal: number
68+
error: { code: string, message: string } | null
69+
createdAt: string
70+
startedAt: string | null
71+
endedAt: string | null
72+
}
73+
74+
/**
75+
* The background dispatch of the test session's check runs failed (or the
76+
* worker died — errorCode ABANDONED): no results will arrive for the
77+
* undispatched checks, so the run should fail immediately instead of waiting
78+
* out the per-check result timeouts.
79+
*/
80+
export class TestSessionSchedulingFailedError extends Error {
81+
/** The operation's error code (e.g. SCHEDULING_ERROR, ABANDONED), when known. */
82+
code?: string
83+
84+
constructor (message: string, options?: ErrorOptions & { code?: string }) {
85+
super(message, options)
86+
this.name = 'TestSessionSchedulingFailedError'
87+
this.code = options?.code
88+
}
89+
}
90+
5291
export type TestSessionMetadata = {
5392
environment?: string
5493
repoUrl?: string
@@ -157,11 +196,42 @@ class TestSessions {
157196
}
158197

159198
async run (payload: RunTestSessionRequest) {
160-
return await this.api.post('/next/test-sessions/run', payload, {
199+
return await this.api.post<RunTestSessionResponse>('/v1/test-sessions/run', payload, {
161200
transformRequest: compressJSONPayload,
162201
})
163202
}
164203

204+
/**
205+
* Long-poll the dispatch status of an asynchronously started test-session
206+
* run. The server blocks for up to `maxWaitSeconds` and returns the
207+
* operation once it reaches a terminal state, or responds 408
208+
* ({@link RequestTimeoutError}) while dispatch is still in progress — the
209+
* retry cadence lives in the caller.
210+
*/
211+
getSchedulingCompletion (schedulingId: string, options?: { maxWaitSeconds?: number, signal?: AbortSignal }) {
212+
return this.api.get<TestSessionSchedulingOperation>(`/v1/test-sessions/scheduling/${schedulingId}/completion`, {
213+
params: { maxWaitSeconds: options?.maxWaitSeconds ?? COMPLETION_MAX_WAIT_SECONDS },
214+
signal: options?.signal,
215+
})
216+
}
217+
218+
async pollSchedulingUntilComplete (
219+
schedulingId: string,
220+
options?: { signal?: AbortSignal },
221+
): Promise<TestSessionSchedulingOperation> {
222+
while (true) {
223+
try {
224+
const { data } = await this.getSchedulingCompletion(schedulingId, options)
225+
return data
226+
} catch (err) {
227+
if (err instanceof RequestTimeoutError && !options?.signal?.aborted) {
228+
continue
229+
}
230+
throw err
231+
}
232+
}
233+
}
234+
165235
/**
166236
* @throws {NoMatchingChecksError} If no checks matched the request.
167237
*/

packages/cli/src/services/__tests__/abstract-check-runner.spec.ts

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ vi.mock('../../rest/api.js', () => ({
99
testSessions: {
1010
run: vi.fn().mockResolvedValue({ data: { testSessionId: 'ts-123', sequenceIds: {} } }),
1111
getResultShortLinks: vi.fn().mockResolvedValue({ data: {} }),
12+
pollSchedulingUntilComplete: vi.fn(),
1213
},
1314
assets: {
1415
getLogs: vi.fn().mockResolvedValue([]),
@@ -32,6 +33,8 @@ vi.mock('../socket-client.js', () => ({
3233
// ---------------------------------------------------------------------------
3334

3435
import { SocketClient } from '../socket-client.js'
36+
import { testSessions } from '../../rest/api.js'
37+
import { TestSessionSchedulingFailedError } from '../../rest/test-sessions.js'
3538

3639
/** Minimal concrete subclass — scheduleChecks immediately returns with zero checks so the runner exits cleanly. */
3740
class StubCheckRunner extends AbstractCheckRunner {
@@ -240,3 +243,135 @@ describe('AbstractCheckRunner — SocketClient lifecycle', () => {
240243
expect(mockClient.endAsync).toHaveBeenCalledTimes(1)
241244
})
242245
})
246+
247+
// ---------------------------------------------------------------------------
248+
// Scheduling watch
249+
// ---------------------------------------------------------------------------
250+
251+
/** Returns a schedulingId and one pending check, so the run only settles via a
252+
* check result or a scheduling failure. */
253+
class SchedulingStubRunner extends AbstractCheckRunner {
254+
checksToSchedule: Array<{ check: any, sequenceId: SequenceId }>
255+
256+
constructor (checks: Array<{ check: any, sequenceId: SequenceId }>) {
257+
super('acc-1', 60, false, false)
258+
this.checksToSchedule = checks
259+
}
260+
261+
// eslint-disable-next-line @typescript-eslint/no-unused-vars
262+
scheduleChecks (_checkRunSuiteId: string): Promise<{
263+
testSessionId?: string
264+
schedulingId?: string
265+
checks: Array<{ check: any, sequenceId: SequenceId }>
266+
}> {
267+
return Promise.resolve({ testSessionId: 'ts-stub', schedulingId: 'op-1', checks: this.checksToSchedule })
268+
}
269+
}
270+
271+
describe('AbstractCheckRunner — scheduling watch', () => {
272+
beforeEach(() => {
273+
vi.clearAllMocks()
274+
vi.mocked(SocketClient.connect).mockResolvedValue({
275+
on: vi.fn(),
276+
subscribeAsync: vi.fn().mockResolvedValue(undefined),
277+
endAsync: vi.fn().mockResolvedValue(undefined),
278+
} as any)
279+
vi.spyOn(process, 'rawListeners').mockReturnValue([])
280+
vi.spyOn(process, 'removeAllListeners').mockReturnValue(process)
281+
vi.spyOn(process, 'on').mockReturnValue(process)
282+
vi.spyOn(process, 'off').mockReturnValue(process)
283+
})
284+
285+
afterEach(() => {
286+
vi.restoreAllMocks()
287+
})
288+
289+
it('fails the run immediately when the scheduling operation reports FAILED', async () => {
290+
vi.mocked(testSessions.pollSchedulingUntilComplete).mockResolvedValue({
291+
schedulingId: 'op-1',
292+
testSessionId: 'ts-stub',
293+
status: 'FAILED',
294+
checksTotal: 1,
295+
error: { code: 'SCHEDULING_ERROR', message: 'Unable to find private location' },
296+
createdAt: '2026-01-01T00:00:00.000Z',
297+
startedAt: null,
298+
endedAt: null,
299+
})
300+
301+
const runner = new SchedulingStubRunner([{ check: {}, sequenceId: 'seq-1' }])
302+
const errors: Error[] = []
303+
runner.on(Events.ERROR, err => errors.push(err))
304+
305+
await runner.run()
306+
307+
expect(errors).toHaveLength(1)
308+
expect(errors[0]).toBeInstanceOf(TestSessionSchedulingFailedError)
309+
expect(errors[0].message).toEqual('Unable to find private location')
310+
expect(testSessions.pollSchedulingUntilComplete).toHaveBeenCalledWith('op-1', expect.objectContaining({
311+
signal: expect.any(AbortSignal),
312+
}))
313+
})
314+
315+
it('does not settle the run when the scheduling operation succeeds', async () => {
316+
vi.mocked(testSessions.pollSchedulingUntilComplete).mockResolvedValue({
317+
schedulingId: 'op-1',
318+
testSessionId: 'ts-stub',
319+
status: 'SUCCEEDED',
320+
checksTotal: 1,
321+
error: null,
322+
createdAt: '2026-01-01T00:00:00.000Z',
323+
startedAt: null,
324+
endedAt: null,
325+
})
326+
327+
// One pending check: the run may only settle once that check finishes —
328+
// the SUCCEEDED watch must neither error nor resolve the race early.
329+
const runner = new SchedulingStubRunner([{ check: {}, sequenceId: 'seq-1' }])
330+
const errors: Error[] = []
331+
let finished = false
332+
runner.on(Events.ERROR, err => errors.push(err))
333+
runner.on(Events.RUN_FINISHED, () => {
334+
finished = true
335+
})
336+
337+
const runPromise = runner.run()
338+
// Let the (mock-resolved) scheduling watch settle before the check does.
339+
await new Promise(resolve => setTimeout(resolve, 20))
340+
expect(finished).toBe(false)
341+
runner.emit(Events.CHECK_FINISHED)
342+
await runPromise
343+
;(runner as any).disableAllTimeouts()
344+
345+
expect(errors).toHaveLength(0)
346+
expect(finished).toBe(true)
347+
})
348+
349+
it('fails a detached run when dispatch fails', async () => {
350+
vi.mocked(testSessions.pollSchedulingUntilComplete).mockResolvedValue({
351+
schedulingId: 'op-1',
352+
testSessionId: 'ts-stub',
353+
status: 'FAILED',
354+
checksTotal: 1,
355+
error: { code: 'ABANDONED', message: 'the worker stopped reporting progress' },
356+
createdAt: '2026-01-01T00:00:00.000Z',
357+
startedAt: null,
358+
endedAt: null,
359+
})
360+
361+
const runner = new SchedulingStubRunner([{ check: {}, sequenceId: 'seq-1' }])
362+
;(runner as any).detach = true
363+
const errors: Array<Error & { code?: string }> = []
364+
let detached = false
365+
runner.on(Events.ERROR, err => errors.push(err))
366+
runner.on(Events.DETACH, () => {
367+
detached = true
368+
})
369+
370+
await runner.run()
371+
372+
expect(detached).toBe(false)
373+
expect(errors).toHaveLength(1)
374+
expect(errors[0]).toBeInstanceOf(TestSessionSchedulingFailedError)
375+
expect(errors[0].code).toEqual('ABANDONED')
376+
})
377+
})

0 commit comments

Comments
 (0)