From 58d1f09b8ea0a6cd90bdc7e9c17b7095200c0b0b Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Fri, 25 Sep 2026 14:27:21 +0200 Subject: [PATCH] Add JSON progress and retry support to task runners --- .changeset/render-tasks-json-progress.md | 5 + docs/cli/json-output.md | 18 + .../src/private/node/ui/components/Tasks.tsx | 63 +-- .../node/ui/hooks/use-async-and-unmount.ts | 14 +- packages/cli-kit/src/private/node/ui/tasks.ts | 96 +++++ .../src/public/common/command-events.test.ts | 11 +- .../src/public/common/command-events.ts | 2 +- .../src/public/node/command-events.test.ts | 20 +- packages/cli-kit/src/public/node/ui.test.ts | 359 +++++++++++++++++- packages/cli-kit/src/public/node/ui.tsx | 66 +++- 10 files changed, 571 insertions(+), 83 deletions(-) create mode 100644 .changeset/render-tasks-json-progress.md create mode 100644 packages/cli-kit/src/private/node/ui/tasks.ts diff --git a/.changeset/render-tasks-json-progress.md b/.changeset/render-tasks-json-progress.md new file mode 100644 index 00000000000..2d3f9da3ee6 --- /dev/null +++ b/.changeset/render-tasks-json-progress.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli-kit': minor +--- + +Run `renderTasks` without Ink in JSON mode, report task retries and failures, and support retries in `renderSingleTask`. diff --git a/docs/cli/json-output.md b/docs/cli/json-output.md index 4829519e073..fe7e9d19354 100644 --- a/docs/cli/json-output.md +++ b/docs/cli/json-output.md @@ -81,6 +81,21 @@ Events are separate from finite results. Progress events can drive spinners or s running, but they aren't fields in the final JSON result. Errors continue through the standard CLI error path; don't encode failures as successful result shapes merely to support `--json`. +### Task progress events + +`renderTasks` uses one `operation` ID for the whole task list, including subtasks. It emits `started` for the first +task that runs, `updated` for subsequent tasks, and `completed` after the whole list succeeds. Skipped tasks emit no +progress events. Empty lists and lists where every task is skipped emit no events. + +`renderTasks` accepts a `retry` count on each task, and `renderSingleTask` accepts it in its options. It is the number +of additional attempts after a failure and defaults to zero. Both emit `retrying` before each repeated task attempt, +using the same operation ID. Once retries are exhausted, they emit one `failed` event and throw the original error. +Only successful operations emit `completed`. Failure events identify the task through `message`; error details +continue through the standard CLI error path. + +Cancellation does not trigger retries or a `failed` event in `renderSingleTask` when its `onAbort` callback runs. +An interrupted operation can still end without a terminal progress event, so consumers must also handle process exit. + ## Preserve compatibility Treat the JSON result as a public API. Keep existing keys, omission rules, nullability, collection shapes, and exit @@ -147,6 +162,9 @@ migration and streaming commands, and remove finite entries as they adopt the co Plugins must adopt the result contract and control their output before their commands can be used reliably in JSON mode. Inheriting `--json-schema` or enabling `SHOPIFY_FLAG_JSON=1` doesn't convert all plugin output automatically. +- In the command event context, `renderTasks` and `renderSingleTask` run without Ink and emit JSON progress events + when JSON mode is enabled. This also applies when `SHOPIFY_FLAG_JSON=1` enables JSON mode for a plugin command that + doesn't declare a `--json` flag. - Oclif `init` hooks run before the command's error handling. A hook that renders a warning and calls `process.exit(1)` bypasses the JSON fatal error path and can leave stdout empty. Put command validation in the command lifecycle and throw an `AbortError` so CLI Kit can encode the failure. diff --git a/packages/cli-kit/src/private/node/ui/components/Tasks.tsx b/packages/cli-kit/src/private/node/ui/components/Tasks.tsx index fd071e06490..62dddeec038 100644 --- a/packages/cli-kit/src/private/node/ui/components/Tasks.tsx +++ b/packages/cli-kit/src/private/node/ui/components/Tasks.tsx @@ -4,19 +4,11 @@ import {isUnitTest} from '../../../../public/node/context/local.js' import {AbortSignal} from '../../../../public/node/abort.js' import useAbortSignal from '../hooks/use-abort-signal.js' import {useExitOnCtrlC} from '../hooks/use-exit-on-ctrl-c.js' -import {TokenizedString} from '../../../../public/node/output.js' +import {runTasks, Task} from '../tasks.js' -import React, {useRef, useState} from 'react' +import React, {useState} from 'react' -export interface Task { - title: string | TokenizedString - - task: (ctx: TContext, task: Task) => Promise[]> - retry?: number - retryCount?: number - errors?: Error[] - skip?: (ctx: TContext) => boolean -} +export type {Task} from '../tasks.js' interface TasksProps { tasks: Task[] @@ -33,30 +25,6 @@ enum TasksState { Failure = 'failure', } -async function runTask(task: Task, ctx: TContext) { - task.retryCount = 0 - task.errors = [] - const retry = task.retry && task.retry > 0 ? task.retry + 1 : 1 - - for (let retries = 1; retries <= retry; retries++) { - try { - if (task.skip?.(ctx)) { - return - } - // eslint-disable-next-line no-await-in-loop - return await task.task(ctx, task) - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } catch (error: any) { - if (retries === retry) { - throw error - } else { - task.errors.push(error) - task.retryCount = retries - } - } - } -} - const noop = () => {} function Tasks({ @@ -69,30 +37,11 @@ function Tasks({ }: React.PropsWithChildren>) { const [currentTask, setCurrentTask] = useState>(tasks[0]!) const [state, setState] = useState(TasksState.Loading) - const ctx = useRef({} as TContext) - - const runTasks = async () => { - for (const task of tasks) { - setCurrentTask(task) - - // eslint-disable-next-line no-await-in-loop - const subTasks = await runTask(task, ctx.current) - - // subtasks - if (Array.isArray(subTasks) && subTasks.length > 0 && subTasks.every((task) => 'task' in task)) { - for (const subTask of subTasks) { - setCurrentTask(subTask) - // eslint-disable-next-line no-await-in-loop - await runTask(subTask, ctx.current) - } - } - } - } - useAsyncAndUnmount(runTasks, { - onFulfilled: () => { + useAsyncAndUnmount(() => runTasks(tasks, setCurrentTask), { + onFulfilled: (context) => { setState(TasksState.Success) - onComplete(ctx.current) + onComplete(context) }, onRejected: () => { setState(TasksState.Failure) diff --git a/packages/cli-kit/src/private/node/ui/hooks/use-async-and-unmount.ts b/packages/cli-kit/src/private/node/ui/hooks/use-async-and-unmount.ts index 9ba9e5ef26e..ce62c886729 100644 --- a/packages/cli-kit/src/private/node/ui/hooks/use-async-and-unmount.ts +++ b/packages/cli-kit/src/private/node/ui/hooks/use-async-and-unmount.ts @@ -1,22 +1,22 @@ import {useComplete} from '../../ui.js' import {useEffect, useState} from 'react' -interface Options { - onFulfilled?: () => unknown +interface Options { + onFulfilled?: (result: T) => unknown onRejected?: (error: Error) => void } -export default function useAsyncAndUnmount( - asyncFunction: () => Promise, - {onFulfilled = () => {}, onRejected = () => {}}: Options = {}, +export default function useAsyncAndUnmount( + asyncFunction: () => Promise, + {onFulfilled = () => {}, onRejected = () => {}}: Options = {}, ) { const complete = useComplete() const [result, setResult] = useState<{error?: Error} | null>(null) useEffect(() => { asyncFunction() - .then(() => { - onFulfilled() + .then((result) => { + onFulfilled(result) setResult({}) }) .catch((error) => { diff --git a/packages/cli-kit/src/private/node/ui/tasks.ts b/packages/cli-kit/src/private/node/ui/tasks.ts new file mode 100644 index 00000000000..3266f82b281 --- /dev/null +++ b/packages/cli-kit/src/private/node/ui/tasks.ts @@ -0,0 +1,96 @@ +import {emitCommandEvent} from '../../../public/node/command-events.js' +import {randomUUID} from '../../../public/node/crypto.js' +import {TokenizedString, unstyled} from '../../../public/node/output.js' + +export interface Task { + title: string | TokenizedString + task: (ctx: TContext, task: Task) => Promise[]> + retry?: number + retryCount?: number + errors?: Error[] + skip?: (ctx: TContext) => boolean +} + +export async function runTasks(tasks: Task[], onTask: (task: Task) => void = () => {}) { + const context = {} as TContext + const operation = randomUUID() + let currentTask: Task | undefined + + const execute = async (task: Task) => { + try { + onTask(task) + return await runTask(task, context, (status) => { + emitCommandEvent( + { + type: 'progress', + operation, + status: status === 'started' && currentTask ? 'updated' : status, + message: taskMessage(task), + }, + {alreadyRendered: true}, + ) + currentTask = task + }) + } catch (error) { + emitCommandEvent( + {type: 'progress', operation, status: 'failed', message: taskMessage(task)}, + {alreadyRendered: true}, + ) + throw error + } + } + + for (const task of tasks) { + // eslint-disable-next-line no-await-in-loop + const subTasks = await execute(task) + + if (Array.isArray(subTasks) && subTasks.length > 0 && subTasks.every((task) => 'task' in task)) { + for (const subTask of subTasks) { + // eslint-disable-next-line no-await-in-loop + await execute(subTask) + } + } + } + + // A task list can grow while it runs, so only the entire list marks the operation complete. + if (currentTask) { + emitCommandEvent( + {type: 'progress', operation, status: 'completed', message: taskMessage(currentTask), current: 1, total: 1}, + {alreadyRendered: true}, + ) + } + + return context +} + +async function runTask( + task: Task, + context: TContext, + onProgress: (status: 'started' | 'retrying') => void, +) { + task.retryCount = 0 + task.errors = [] + const maxAttempts = task.retry && task.retry > 0 ? task.retry + 1 : 1 + let started = false + + for (let attempt = 1; attempt <= maxAttempts; attempt++) { + try { + if (task.skip?.(context)) return + + onProgress(started ? 'retrying' : 'started') + started = true + + // eslint-disable-next-line no-await-in-loop + return await task.task(context, task) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } catch (error: any) { + if (attempt === maxAttempts) throw error + task.errors.push(error) + task.retryCount = attempt + } + } +} + +function taskMessage(task: Task) { + return unstyled(typeof task.title === 'string' ? task.title : task.title.value) +} diff --git a/packages/cli-kit/src/public/common/command-events.test.ts b/packages/cli-kit/src/public/common/command-events.test.ts index 0a5a77ed590..8a6ad2028f3 100644 --- a/packages/cli-kit/src/public/common/command-events.test.ts +++ b/packages/cli-kit/src/public/common/command-events.test.ts @@ -38,11 +38,14 @@ describe('commandEventSchema', () => { expect(commandEventSchema.parse(event)).toEqual(event) }) - test.each(['started', 'updated', 'completed'])('accepts %s progress without a message', (status) => { - const event = {type: 'progress', timestamp: '2026-08-26T12:00:00.000Z', operation: 'upload', status} + test.each(['started', 'updated', 'retrying', 'completed', 'failed'])( + 'accepts %s progress without a message', + (status) => { + const event = {type: 'progress', timestamp: '2026-08-26T12:00:00.000Z', operation: 'upload', status} - expect(commandEventSchema.parse(event)).toEqual(event) - }) + expect(commandEventSchema.parse(event)).toEqual(event) + }, + ) test.each([{operation: 'upload'}, {status: 'started'}, {operation: 'upload', status: 'unknown'}])( 'rejects incomplete or invalid progress metadata: %j', diff --git a/packages/cli-kit/src/public/common/command-events.ts b/packages/cli-kit/src/public/common/command-events.ts index f94c142ba38..cf12aefd7ce 100644 --- a/packages/cli-kit/src/public/common/command-events.ts +++ b/packages/cli-kit/src/public/common/command-events.ts @@ -16,7 +16,7 @@ export const commandProgressEventSchema = z .object({ type: z.literal('progress'), timestamp: z.string().datetime({offset: true}), - status: z.enum(['started', 'updated', 'completed']), + status: z.enum(['started', 'updated', 'retrying', 'completed', 'failed']), operation: z.string(), message: z.string().optional(), current: z.number().nonnegative().optional(), diff --git a/packages/cli-kit/src/public/node/command-events.test.ts b/packages/cli-kit/src/public/node/command-events.test.ts index 2bc7042787c..05d6960c332 100644 --- a/packages/cli-kit/src/public/node/command-events.test.ts +++ b/packages/cli-kit/src/public/node/command-events.test.ts @@ -29,7 +29,11 @@ describe('commandEventOutputSchema', () => { additionalProperties: false, }, CommandProgressEvent: { - properties: {current: {type: 'number', minimum: 0}, total: {type: 'number', minimum: 0}}, + properties: { + status: {enum: ['started', 'updated', 'retrying', 'completed', 'failed']}, + current: {type: 'number', minimum: 0}, + total: {type: 'number', minimum: 0}, + }, required: ['type', 'timestamp', 'status', 'operation'], additionalProperties: false, }, @@ -131,6 +135,20 @@ describe('renderCommandEvent', () => { }) describe('renderCommandEventAsJson', () => { + test.each(['retrying', 'failed'] as const)('renders %s progress as JSON', (status) => { + const event: CommandEvent = { + type: 'progress', + timestamp: '2026-08-26T12:00:00.000Z', + operation: 'upload', + status, + message: 'Uploading files', + } + + renderCommandEventAsJson(event) + + expect(JSON.parse(outputMock.info())).toEqual(event) + }) + test.each([ {type: 'diagnostic', level: 'unknown', message: 'Invalid level'}, {type: 'progress', operation: 'upload', status: 'started', current: -1}, diff --git a/packages/cli-kit/src/public/node/ui.test.ts b/packages/cli-kit/src/public/node/ui.test.ts index cc6de0f34b9..0276f4cae32 100644 --- a/packages/cli-kit/src/public/node/ui.test.ts +++ b/packages/cli-kit/src/public/node/ui.test.ts @@ -7,12 +7,14 @@ import { renderTasks, renderWarning, renderSingleTask, + Task, } from './ui.js' import {AbortSignal} from './abort.js' import {BugError, FatalError, AbortError, FatalErrorType} from './error.js' -import {runWithCommandEvents} from './command-events.js' -import {mockAndCaptureOutput} from './testing/output.js' +import {renderCommandEventAsJson, runWithCommandEvents} from './command-events.js' +import {mockAndCaptureOutput, withCapturedStandardStreams} from './testing/output.js' import {TokenizedString} from './output.js' +import {Stdin} from '../../private/node/testing/ui.js' import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest' import supportsHyperlinks from 'supports-hyperlinks' @@ -340,6 +342,232 @@ describe('renderConcurrent', async () => { }) describe('renderTasks', async () => { + test.each(['text', 'json'] as const)( + 'preserves context, retries, subtasks, and skipping in %s mode', + async (outputMode) => { + const sink = vi.fn() + const error = new Error('Try again') + const skipped = vi.fn() + const tasks: Task<{steps: string[]}>[] = [ + { + title: 'Prepare', + task: async (context) => { + context.steps = ['prepare'] + }, + }, + { + title: 'Upload', + retry: 1, + task: async (context, task) => { + if (task.retryCount === 0) throw error + expect(task.errors).toEqual([error]) + context.steps.push('upload') + return [ + {title: 'Skipped subtask', skip: () => true, task: skipped}, + { + title: 'Verify', + task: async (context) => { + context.steps.push('verify') + }, + }, + ] + }, + }, + {title: 'Skipped task', skip: (context) => context.steps.includes('verify'), task: skipped}, + ] + + const context = await runWithCommandEvents({sink, outputMode}, () => renderTasks(tasks)) + + expect(context).toEqual({steps: ['prepare', 'upload', 'verify']}) + expect(skipped).not.toHaveBeenCalled() + expect(tasks[1]!.retryCount).toBe(1) + const events = sink.mock.calls.map(([event]) => event) + expect(events.map(({status, message}) => ({status, message}))).toEqual([ + {status: 'started', message: 'Prepare'}, + {status: 'updated', message: 'Upload'}, + {status: 'retrying', message: 'Upload'}, + {status: 'updated', message: 'Verify'}, + {status: 'completed', message: 'Verify'}, + ]) + expect(events[0].operation).toEqual(expect.any(String)) + expect(new Set(events.map(({operation}) => operation)).size).toBe(1) + expect(events.at(-1)).toMatchObject({current: 1, total: 1}) + expect(sink.mock.calls.every(([, options]) => options.alreadyRendered)).toBe(true) + }, + ) + + test('writes JSON progress to stderr without rendering terminal UI', async () => { + const write = vi.fn((_chunk, _encoding, callback) => callback()) + const stdout = new Writable({write}) + await withCapturedStandardStreams(async (streams) => { + await runWithCommandEvents({outputMode: 'json', sink: renderCommandEventAsJson}, () => + renderTasks( + [ + {title: '\u001b[32mUpload\u001b[39m', task: async () => {}}, + {title: new TokenizedString('Upload'), task: async () => {}}, + ], + {renderOptions: {stdout: stdout as NodeJS.WriteStream}}, + ), + ) + + expect(write).not.toHaveBeenCalled() + expect(streams.stdout()).toBe('') + const events = streams + .stderr() + .trim() + .split('\n') + .map((line) => JSON.parse(line)) + expect(events).toHaveLength(3) + expect(events.every((event) => event.type === 'progress' && event.message === 'Upload')).toBe(true) + expect(events[0].operation).toBe(events[2].operation) + }) + }) + + test.each(['text', 'json'] as const)('stops after exhausting retries in %s mode', async (outputMode) => { + const sink = vi.fn() + const error = new Error('Upload failed') + const task = vi.fn().mockRejectedValue(error) + const nextTask = vi.fn() + + await expect( + runWithCommandEvents({sink, outputMode}, () => + renderTasks([ + {title: 'Upload', retry: 2, task}, + {title: 'Next', task: nextTask}, + ]), + ), + ).rejects.toBe(error) + + expect(task).toHaveBeenCalledTimes(3) + expect(nextTask).not.toHaveBeenCalled() + const events = sink.mock.calls.map(([event]) => event) + expect(events.map(({status, message}) => ({status, message}))).toEqual([ + {status: 'started', message: 'Upload'}, + {status: 'retrying', message: 'Upload'}, + {status: 'retrying', message: 'Upload'}, + {status: 'failed', message: 'Upload'}, + ]) + expect(new Set(events.map(({operation}) => operation)).size).toBe(1) + expect(sink.mock.calls.every(([, options]) => options.alreadyRendered)).toBe(true) + }) + + test.each(['text', 'json'] as const)('reports failure without retrying by default in %s mode', async (outputMode) => { + const sink = vi.fn() + const error = new Error('Upload failed') + const task = vi.fn().mockRejectedValue(error) + + await expect(runWithCommandEvents({sink, outputMode}, () => renderTasks([{title: 'Upload', task}]))).rejects.toBe( + error, + ) + + expect(task).toHaveBeenCalledOnce() + expect(sink.mock.calls.map(([event]) => event.status)).toEqual(['started', 'failed']) + }) + + test.each(['text', 'json'] as const)( + 'fails the operation when a subtask exhausts retries in %s mode', + async (outputMode) => { + const sink = vi.fn() + const error = new Error('Verification failed') + const subtask = vi.fn().mockRejectedValueOnce(new Error('Try again')).mockRejectedValue(error) + const nextTask = vi.fn() + + await expect( + runWithCommandEvents({sink, outputMode}, () => + renderTasks([ + { + title: 'Upload', + task: async () => [ + {title: new TokenizedString('\u001b[32mVerify\u001b[39m'), retry: 1, task: subtask}, + {title: 'Next subtask', task: nextTask}, + ], + }, + {title: 'Next task', task: nextTask}, + ]), + ), + ).rejects.toBe(error) + + expect(subtask).toHaveBeenCalledTimes(2) + expect(nextTask).not.toHaveBeenCalled() + const events = sink.mock.calls.map(([event]) => event) + expect(events.map(({status, message}) => ({status, message}))).toEqual([ + {status: 'started', message: 'Upload'}, + {status: 'updated', message: 'Verify'}, + {status: 'retrying', message: 'Verify'}, + {status: 'failed', message: 'Verify'}, + ]) + expect(new Set(events.map(({operation}) => operation)).size).toBe(1) + }, + ) + + test.each(['text', 'json'] as const)('emits no events when all tasks are skipped in %s mode', async (outputMode) => { + const sink = vi.fn() + const task = vi.fn() + + const context = await runWithCommandEvents({sink, outputMode}, () => + renderTasks([{title: 'Skipped', retry: 1, skip: () => true, task}]), + ) + + expect(context).toEqual({}) + expect(task).not.toHaveBeenCalled() + expect(sink).not.toHaveBeenCalled() + }) + + test.each(['text', 'json'] as const)('runs tasks appended during execution in %s mode', async (outputMode) => { + const sink = vi.fn() + const tasks: Task<{finished: boolean}>[] = [ + { + title: 'Wait', + task: async () => { + tasks.push({ + title: 'Finish', + task: async (context) => { + expect(sink.mock.calls.map(([event]) => event.status)).toEqual(['started', 'updated']) + context.finished = true + }, + }) + }, + }, + ] + + const context = await runWithCommandEvents({outputMode, sink}, () => renderTasks(tasks)) + + expect(context).toEqual({finished: true}) + expect(sink.mock.calls.map(([event]) => event.status)).toEqual(['started', 'updated', 'completed']) + }) + + test('distinguishes concurrent JSON task lists with the same title', async () => { + const sink = vi.fn() + await runWithCommandEvents({outputMode: 'json', sink}, () => + Promise.all([ + renderTasks([{title: 'Upload', task: async () => {}}]), + renderTasks([{title: 'Upload', task: async () => {}}]), + ]), + ) + const events = sink.mock.calls.map(([event]) => event) + const operations = events.filter((event) => event.status === 'started').map((event) => event.operation) + expect(new Set(operations).size).toBe(2) + for (const operation of operations) { + expect(events.filter((event) => event.operation === operation).map((event) => event.status)).toEqual([ + 'started', + 'completed', + ]) + } + }) + + test('returns an empty context without rendering terminal UI for an empty JSON task list', async () => { + const sink = vi.fn() + const write = vi.fn((_chunk, _encoding, callback) => callback()) + const stdout = new Writable({write}) + const context = await runWithCommandEvents({outputMode: 'json', sink}, () => + renderTasks([], {renderOptions: {stdout: stdout as NodeJS.WriteStream}}), + ) + + expect(context).toEqual({}) + expect(sink).not.toHaveBeenCalled() + expect(write).not.toHaveBeenCalled() + }) + test('renders an error message correctly when the task throws an error', async () => { // Given const mockOutput = mockAndCaptureOutput() @@ -425,6 +653,73 @@ describe('keypress', async () => { }) describe('renderSingleTask', async () => { + test.each(['text', 'json'] as const)('returns the result after retries in %s mode', async (outputMode) => { + const sink = vi.fn() + const result = {id: 'store'} + const task = vi + .fn() + .mockImplementationOnce(async (updateStatus) => { + updateStatus(new TokenizedString('\u001b[32mSaving session\u001b[39m')) + throw new Error('Try again') + }) + .mockRejectedValueOnce(new Error('Try once more')) + .mockResolvedValue(result) + + await expect( + runWithCommandEvents({sink, outputMode}, () => + renderSingleTask({title: new TokenizedString('Creating store'), retry: 2, task}), + ), + ).resolves.toBe(result) + + expect(task).toHaveBeenCalledTimes(3) + const events = sink.mock.calls.map(([event]) => event) + expect(events.map(({status, message}) => ({status, message}))).toEqual([ + {status: 'started', message: 'Creating store'}, + {status: 'updated', message: 'Saving session'}, + {status: 'retrying', message: 'Saving session'}, + {status: 'retrying', message: 'Saving session'}, + {status: 'completed', message: 'Saving session'}, + ]) + expect(new Set(events.map(({operation}) => operation)).size).toBe(1) + expect(events.at(-1)).toMatchObject({current: 1, total: 1}) + expect(sink.mock.calls.every(([, options]) => options.alreadyRendered)).toBe(true) + }) + + test.each(['text', 'json'] as const)('reports the final failure after retries in %s mode', async (outputMode) => { + const sink = vi.fn() + const error = new Error('Upload failed') + const task = vi.fn().mockRejectedValueOnce(new Error('Try again')).mockRejectedValue(error) + + await expect( + runWithCommandEvents({sink, outputMode}, () => + renderSingleTask({title: new TokenizedString('Uploading files'), retry: 1, task}), + ), + ).rejects.toBe(error) + + expect(task).toHaveBeenCalledTimes(2) + const events = sink.mock.calls.map(([event]) => event) + expect(events.map(({status, message}) => ({status, message}))).toEqual([ + {status: 'started', message: 'Uploading files'}, + {status: 'retrying', message: 'Uploading files'}, + {status: 'failed', message: 'Uploading files'}, + ]) + expect(new Set(events.map(({operation}) => operation)).size).toBe(1) + expect(sink.mock.calls.every(([, options]) => options.alreadyRendered)).toBe(true) + }) + + test.each(['text', 'json'] as const)('reports failure without retrying by default in %s mode', async (outputMode) => { + const sink = vi.fn() + const error = new Error('Upload failed') + const task = vi.fn().mockRejectedValue(error) + + await expect( + runWithCommandEvents({sink, outputMode}, () => renderSingleTask({title: new TokenizedString('Upload'), task})), + ).rejects.toBe(error) + + expect(task).toHaveBeenCalledOnce() + expect(sink.mock.calls.map(([event]) => event.status)).toEqual(['started', 'failed']) + }) + test.each(['text', 'json'] as const)('emits correlated progress in %s mode', async (outputMode) => { const sink = vi.fn() @@ -489,8 +784,9 @@ describe('renderSingleTask', async () => { test('calls onAbort on SIGINT in JSON mode and removes the listener', async () => { const listeners = process.listeners('SIGINT') const onAbort = vi.fn() + const sink = vi.fn() - await runWithCommandEvents({outputMode: 'json'}, () => + await runWithCommandEvents({sink, outputMode: 'json'}, () => renderSingleTask({ title: new TokenizedString('Waiting'), onAbort, @@ -504,6 +800,61 @@ describe('renderSingleTask', async () => { ) expect(process.listeners('SIGINT')).toEqual(listeners) + expect(sink.mock.calls.map(([event]) => event.status)).toEqual(['started']) + }) + + test('does not retry or report failure when a JSON task rejects after cancellation', async () => { + const listeners = process.listeners('SIGINT') + const sink = vi.fn() + const onAbort = vi.fn() + const error = new Error('Cancelled') + const task = vi.fn(async () => { + process.emit('SIGINT') + throw error + }) + + await expect( + runWithCommandEvents({sink, outputMode: 'json'}, () => + renderSingleTask({title: new TokenizedString('Waiting'), retry: 2, onAbort, task}), + ), + ).rejects.toBe(error) + + expect(task).toHaveBeenCalledOnce() + expect(onAbort).toHaveBeenCalledOnce() + expect(sink.mock.calls.map(([event]) => event.status)).toEqual(['started']) + expect(process.listeners('SIGINT')).toEqual(listeners) + }) + + test('does not retry or report failure when a text task rejects after cancellation', async () => { + const sink = vi.fn() + const stdin = new Stdin() + const error = new Error('Cancelled') + let rejectTask: (error: Error) => void + const task = vi.fn( + () => + new Promise((_resolve, reject) => { + rejectTask = reject + }), + ) + const onAbort = vi.fn(() => rejectTask(error)) + + const result = runWithCommandEvents({sink, outputMode: 'text'}, () => + renderSingleTask({ + title: new TokenizedString('Waiting'), + retry: 2, + onAbort, + task, + renderOptions: {stdin: stdin as unknown as NodeJS.ReadStream}, + }), + ) + const rejection = expect(result).rejects.toBe(error) + await vi.waitFor(() => expect(task).toHaveBeenCalledOnce()) + stdin.write('\u0003') + await rejection + + expect(task).toHaveBeenCalledOnce() + expect(onAbort).toHaveBeenCalledOnce() + expect(sink.mock.calls.map(([event]) => event.status)).toEqual(['started']) }) test.each([false, true])('removes the JSON abort listener when the task settles (failure: %s)', async (fails) => { @@ -526,7 +877,7 @@ describe('renderSingleTask', async () => { if (fails) { await expect(result).rejects.toBe(error) - expect(sink.mock.calls.map(([event]) => event.status)).toEqual(['started']) + expect(sink.mock.calls.map(([event]) => event.status)).toEqual(['started', 'failed']) } else { await expect(result).resolves.toBe('done') } diff --git a/packages/cli-kit/src/public/node/ui.tsx b/packages/cli-kit/src/public/node/ui.tsx index 774cd0203ba..ad2bda3c567 100644 --- a/packages/cli-kit/src/public/node/ui.tsx +++ b/packages/cli-kit/src/public/node/ui.tsx @@ -27,6 +27,7 @@ import { } from '../../private/node/ui/components/DangerousConfirmationPrompt.js' import {SelectPrompt, SelectPromptProps} from '../../private/node/ui/components/SelectPrompt.js' import {Tasks, Task} from '../../private/node/ui/components/Tasks.js' +import {runTasks} from '../../private/node/ui/tasks.js' import {TextPrompt, TextPromptProps} from '../../private/node/ui/components/TextPrompt.js' import {AutocompletePromptProps, AutocompletePrompt} from '../../private/node/ui/components/AutocompletePrompt.js' import {InfoTableSection} from '../../private/node/ui/components/Prompts/InfoTable.js' @@ -490,6 +491,8 @@ export async function renderTasks( tasks: Task[], {renderOptions, noProgressBar}: RenderTasksOptions = {}, ): Promise { + if (commandEventOutputMode() === 'json') return runTasks(tasks) + let taskResult: TContext await render( ( export interface RenderSingleTaskOptions { title: TokenizedString task: (updateStatus: (status: TokenizedString) => void) => Promise + /** The number of additional attempts after a failure. Defaults to zero. */ + retry?: number onAbort?: () => void renderOptions?: RenderOptions } @@ -520,6 +525,7 @@ export interface RenderSingleTaskOptions { * @param options - Configuration object * @param options.title - The initial title to display with the loading bar * @param options.task - The async task to execute. Receives an updateStatus callback to change the displayed title. + * @param options.retry - The number of additional attempts after a failure. Defaults to zero. * @param options.renderOptions - Optional render configuration * @returns The result of the task * @example @@ -528,36 +534,78 @@ export interface RenderSingleTaskOptions { export async function renderSingleTask({ title, task, + retry = 0, onAbort, renderOptions, }: RenderSingleTaskOptions): Promise { // Keep updates correlated even when titles change or concurrent tasks share the same title. const operation = randomUUID() let currentStatus = title + let aborted = false + const abort = onAbort + ? () => { + aborted = true + onAbort() + } + : undefined const taskWithProgressEvents = async (updateStatus: (status: TokenizedString) => void): Promise => { emitCommandEvent( {type: 'progress', operation, status: 'started', message: unstyled(currentStatus.value)}, {alreadyRendered: true}, ) - const result = await task((status) => { + const updateTaskStatus = (status: TokenizedString) => { currentStatus = status emitCommandEvent( {type: 'progress', operation, status: 'updated', message: unstyled(status.value)}, {alreadyRendered: true}, ) updateStatus(status) - }) - emitCommandEvent( - {type: 'progress', operation, status: 'completed', message: unstyled(currentStatus.value), current: 1, total: 1}, - {alreadyRendered: true}, - ) - return result + } + + for (let attempt = 0; ; attempt++) { + let result: T + try { + // eslint-disable-next-line no-await-in-loop + result = await task(updateTaskStatus) + } catch (error) { + // A custom abort callback can reject the task; cancellation must not start another attempt. + if (aborted) throw error + + const shouldRetry = attempt < retry + emitCommandEvent( + { + type: 'progress', + operation, + status: shouldRetry ? 'retrying' : 'failed', + message: unstyled(currentStatus.value), + }, + {alreadyRendered: true}, + ) + if (!shouldRetry) throw error + continue + } + + if (!aborted) { + emitCommandEvent( + { + type: 'progress', + operation, + status: 'completed', + message: unstyled(currentStatus.value), + current: 1, + total: 1, + }, + {alreadyRendered: true}, + ) + } + return result + } } if (commandEventOutputMode() === 'json') { // Without Ink's raw input handling, Ctrl+C arrives as SIGINT. Leave the default // signal behavior intact when the caller has no custom abort callback. - const onSigint = () => onAbort?.() + const onSigint = () => abort?.() if (onAbort) process.once('SIGINT', onSigint) try { return await taskWithProgressEvents(() => {}) @@ -574,7 +622,7 @@ export async function renderSingleTask({ onComplete={(result) => { taskResult = result }} - onAbort={onAbort} + onAbort={abort} />, { stdout: process.stderr as unknown as NodeJS.WriteStream,