diff --git a/src/commands/contact-properties/update.ts b/src/commands/contact-properties/update.ts index f0e11d25..1f18d706 100644 --- a/src/commands/contact-properties/update.ts +++ b/src/commands/contact-properties/update.ts @@ -79,7 +79,7 @@ The fallback value is used in broadcast template interpolation when a contact ha if (typeof fallbackValue === 'string') { const resend = await requireClient(globalOpts); const property = await withSpinner( - 'Fetching contact property...', + { loading: 'Fetching contact property...', retryTransient: true }, () => resend.contactProperties.get(id), 'fetch_error', globalOpts, diff --git a/src/commands/contacts/update-topics.ts b/src/commands/contacts/update-topics.ts index e10a80cb..c634d297 100644 --- a/src/commands/contacts/update-topics.ts +++ b/src/commands/contacts/update-topics.ts @@ -77,7 +77,7 @@ Topics not included in the array are left unchanged.`, // contactIdentifier's result is directly assignable: UpdateContactTopicsBaseOptions // uses optional { id?, email? } (not a discriminated union). const data = await withSpinner( - 'Updating topic subscriptions...', + { loading: 'Updating topic subscriptions...' }, () => resend.contacts.topics.update({ ...contactIdentifier(id), topics }), 'update_topics_error', globalOpts, diff --git a/src/commands/emails/batch.ts b/src/commands/emails/batch.ts index 170b343c..dc00774b 100644 --- a/src/commands/emails/batch.ts +++ b/src/commands/emails/batch.ts @@ -142,7 +142,7 @@ export const batchCommand = new Command('batch') } const batchData = await withSpinner( - 'Sending batch...', + { loading: 'Sending batch...' }, () => { const options = { ...(opts.idempotencyKey && { idempotencyKey: opts.idempotencyKey }), diff --git a/src/commands/emails/send.ts b/src/commands/emails/send.ts index 7161ba5d..ce4d4f2a 100644 --- a/src/commands/emails/send.ts +++ b/src/commands/emails/send.ts @@ -396,7 +396,9 @@ export const sendCommand = new Command('send') }); const data = await withSpinner( - opts.scheduledAt ? 'Scheduling email...' : 'Sending email...', + { + loading: opts.scheduledAt ? 'Scheduling email...' : 'Sending email...', + }, () => resend.emails.send( payload, diff --git a/src/commands/templates/publish.ts b/src/commands/templates/publish.ts index b3f654c2..c48dbf7a 100644 --- a/src/commands/templates/publish.ts +++ b/src/commands/templates/publish.ts @@ -31,6 +31,7 @@ Publishing an already-published template re-publishes it with the latest draft c sdkCall: (resend) => resend.templates.publish(id), errorCode: 'publish_error', successMsg: `Template published: ${id}`, + retryTransient: true, }, globalOpts, ); diff --git a/src/commands/templates/update.ts b/src/commands/templates/update.ts index 6a4412be..e33f3954 100644 --- a/src/commands/templates/update.ts +++ b/src/commands/templates/update.ts @@ -159,6 +159,7 @@ export const updateTemplateCommand = new Command('update') }), errorCode: 'update_error', successMsg: `Template updated: ${id}`, + retryTransient: true, }, globalOpts, ); diff --git a/src/lib/actions.ts b/src/lib/actions.ts index 41d85ee3..ec2a782a 100644 --- a/src/lib/actions.ts +++ b/src/lib/actions.ts @@ -12,10 +12,6 @@ type SdkCall = ( resend: Resend, ) => Promise<{ data: T | null; error: { message: string } | null }>; -/** - * Shared pattern for all get commands: - * requireClient → withSpinner(fetch_error) → if/else output - */ export async function runGet( config: { loading: string; @@ -30,7 +26,7 @@ export async function runGet( : undefined; const resend = await requireClient(globalOpts, clientOpts); const data = await withSpinner( - config.loading, + { loading: config.loading, retryTransient: true }, () => config.sdkCall(resend), 'fetch_error', globalOpts, @@ -42,10 +38,6 @@ export async function runGet( } } -/** - * Shared pattern for all delete commands: - * requireClient → confirmDelete (if needed) → withSpinner → if/else output - */ export async function runDelete( id: string, skipConfirm: boolean, @@ -67,7 +59,7 @@ export async function runDelete( await confirmDelete(id, config.confirmMessage, globalOpts); } await withSpinner( - config.loading, + { loading: config.loading }, () => config.sdkCall(resend), 'delete_error', globalOpts, @@ -82,10 +74,6 @@ export async function runDelete( } } -/** - * Shared pattern for create commands: - * requireClient → withSpinner('create_error') → if/else output - */ export async function runCreate( config: { loading: string; @@ -100,7 +88,7 @@ export async function runCreate( : undefined; const resend = await requireClient(globalOpts, clientOpts); const data = await withSpinner( - config.loading, + { loading: config.loading }, () => config.sdkCall(resend), 'create_error', globalOpts, @@ -112,17 +100,13 @@ export async function runCreate( } } -/** - * Shared pattern for write commands (update/verify/remove-segment) where - * interactive output is a single status message: - * requireClient → withSpinner(errorCode) → if/else output - */ export async function runWrite( config: { loading: string; sdkCall: SdkCall; errorCode: string; successMsg: string; + retryTransient?: boolean; permission?: ApiKeyPermission; }, globalOpts: GlobalOpts, @@ -132,7 +116,7 @@ export async function runWrite( : undefined; const resend = await requireClient(globalOpts, clientOpts); const data = await withSpinner( - config.loading, + { loading: config.loading, retryTransient: config.retryTransient }, () => config.sdkCall(resend), config.errorCode, globalOpts, @@ -144,13 +128,6 @@ export async function runWrite( } } -/** - * Shared pattern for all list commands: - * requireClient → withSpinner → if/else output - * - * Callers pass pagination opts (if any) via the sdkCall closure. - * The onInteractive callback handles table rendering and pagination hints. - */ export async function runList( config: { loading: string; @@ -165,7 +142,7 @@ export async function runList( : undefined; const resend = await requireClient(globalOpts, clientOpts); const result = await withSpinner( - config.loading, + { loading: config.loading, retryTransient: true }, () => config.sdkCall(resend), 'list_error', globalOpts, diff --git a/src/lib/is-transient-error.ts b/src/lib/is-transient-error.ts new file mode 100644 index 00000000..67292561 --- /dev/null +++ b/src/lib/is-transient-error.ts @@ -0,0 +1,8 @@ +const TRANSIENT_ERROR_NAMES: ReadonlySet = new Set([ + 'internal_server_error', + 'service_unavailable', + 'gateway_timeout', +]); + +export const isTransientError = (name?: string): boolean => + name != null && TRANSIENT_ERROR_NAMES.has(name); diff --git a/src/lib/spinner.ts b/src/lib/spinner.ts index 88e4134a..268ca202 100644 --- a/src/lib/spinner.ts +++ b/src/lib/spinner.ts @@ -1,5 +1,6 @@ import pc from 'picocolors'; import type { GlobalOpts } from './client'; +import { isTransientError } from './is-transient-error'; import { errorMessage, outputError } from './output'; import { isInteractive, isUnicodeSupported } from './tty'; @@ -49,30 +50,39 @@ function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } -/** - * Wraps an SDK call with a loading spinner and unified error handling. - * - * The spinner is purely a loading indicator — it clears itself when the call - * completes. Callers are responsible for printing success output. - * - * Automatically retries on rate_limit_exceeded errors (HTTP 429) up to 3 times, - * using the retry-after header when available or exponential backoff (1s, 2s, 4s). - */ +type SpinnerMessages = { + readonly loading: string; + readonly retryTransient?: boolean; +}; + +const isRetryable = ( + error: { message: string; name?: string }, + retryTransient: boolean, +): boolean => + error.name === 'rate_limit_exceeded' || + (retryTransient && isTransientError(error.name)); + +const retryMessage = (error: { name?: string }, delay: number): string => + error.name === 'rate_limit_exceeded' + ? `Rate limited, retrying in ${delay}s...` + : `Server error, retrying in ${delay}s...`; + export async function withSpinner( - loading: string, + messages: SpinnerMessages, call: () => Promise>, errorCode: string, globalOpts: GlobalOpts, ): Promise { + const { loading, retryTransient = false } = messages; const spinner = createSpinner(loading, globalOpts.quiet); try { for (let attempt = 0; ; attempt++) { const { data, error, headers } = await call(); if (error) { - if (attempt < MAX_RETRIES && error.name === 'rate_limit_exceeded') { + if (attempt < MAX_RETRIES && isRetryable(error, retryTransient)) { const delay = parseRetryDelay(headers) ?? DEFAULT_RETRY_DELAYS[attempt]; - spinner.update(`Rate limited, retrying in ${delay}s...`); + spinner.update(retryMessage(error, delay)); await sleep(delay * 1000); spinner.update(loading); continue; diff --git a/tests/lib/is-transient-error.test.ts b/tests/lib/is-transient-error.test.ts new file mode 100644 index 00000000..15490ff3 --- /dev/null +++ b/tests/lib/is-transient-error.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from 'vitest'; +import { isTransientError } from '../../src/lib/is-transient-error'; + +describe('isTransientError', () => { + it('returns true for internal_server_error', () => { + expect(isTransientError('internal_server_error')).toBe(true); + }); + + it('returns true for service_unavailable', () => { + expect(isTransientError('service_unavailable')).toBe(true); + }); + + it('returns true for gateway_timeout', () => { + expect(isTransientError('gateway_timeout')).toBe(true); + }); + + it('returns false for rate_limit_exceeded', () => { + expect(isTransientError('rate_limit_exceeded')).toBe(false); + }); + + it('returns false for not_found', () => { + expect(isTransientError('not_found')).toBe(false); + }); + + it('returns false for undefined', () => { + expect(isTransientError(undefined)).toBe(false); + }); +}); diff --git a/tests/lib/spinner.test.ts b/tests/lib/spinner.test.ts index c8a1115c..2abeafe7 100644 --- a/tests/lib/spinner.test.ts +++ b/tests/lib/spinner.test.ts @@ -23,8 +23,6 @@ describe('withSpinner retry on rate_limit_exceeded', () => { const msgs = { loading: 'Loading...', - success: 'Done', - fail: 'Failed', }; const globalOpts = { json: true }; @@ -89,7 +87,6 @@ describe('withSpinner retry on rate_limit_exceeded', () => { expect((err as ExitError).code).toBe(1); } expect(threw).toBe(true); - // 1 initial + 3 retries = 4 total calls expect(calls).toBe(4); }); @@ -160,7 +157,6 @@ describe('withSpinner retry on rate_limit_exceeded', () => { const result = await withSpinner(msgs, call, 'test_error', globalOpts); expect(result).toEqual({ ok: true }); - // retry-after: 0 means near-instant retry expect(Date.now() - start).toBeLessThan(500); }); @@ -184,11 +180,200 @@ describe('withSpinner retry on rate_limit_exceeded', () => { const start = Date.now(); const result = await withSpinner(msgs, call, 'test_error', globalOpts); expect(result).toEqual({ ok: true }); - // Default first retry delay is 1s expect(Date.now() - start).toBeGreaterThanOrEqual(900); }); }); +describe('withSpinner retry on transient 5xx errors', () => { + const restoreEnv = captureTestEnv(); + let exitSpy: MockInstance; + let errorSpy: MockInstance; + let stderrSpy: MockInstance; + + const globalOpts = { json: true }; + + beforeEach(() => { + setNonInteractive(); + exitSpy = mockExitThrow(); + errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + stderrSpy = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); + }); + + afterEach(() => { + restoreEnv(); + exitSpy.mockRestore(); + errorSpy.mockRestore(); + stderrSpy.mockRestore(); + }); + + it('retries internal_server_error when retryTransient is true', async () => { + let calls = 0; + const call = async () => { + calls++; + if (calls === 1) { + return { + data: null, + error: { + message: 'Internal server error', + name: 'internal_server_error', + }, + headers: null, + }; + } + return { data: { id: 'ok' }, error: null, headers: null }; + }; + + const result = await withSpinner( + { loading: 'Loading...', retryTransient: true }, + call, + 'test_error', + globalOpts, + ); + expect(result).toEqual({ id: 'ok' }); + expect(calls).toBe(2); + }); + + it('retries service_unavailable when retryTransient is true', async () => { + let calls = 0; + const call = async () => { + calls++; + if (calls === 1) { + return { + data: null, + error: { + message: 'Service unavailable', + name: 'service_unavailable', + }, + headers: null, + }; + } + return { data: { id: 'ok' }, error: null, headers: null }; + }; + + const result = await withSpinner( + { loading: 'Loading...', retryTransient: true }, + call, + 'test_error', + globalOpts, + ); + expect(result).toEqual({ id: 'ok' }); + expect(calls).toBe(2); + }); + + it('retries gateway_timeout when retryTransient is true', async () => { + let calls = 0; + const call = async () => { + calls++; + if (calls === 1) { + return { + data: null, + error: { message: 'Gateway timeout', name: 'gateway_timeout' }, + headers: null, + }; + } + return { data: { id: 'ok' }, error: null, headers: null }; + }; + + const result = await withSpinner( + { loading: 'Loading...', retryTransient: true }, + call, + 'test_error', + globalOpts, + ); + expect(result).toEqual({ id: 'ok' }); + expect(calls).toBe(2); + }); + + it('does not retry transient errors when retryTransient is false', async () => { + let calls = 0; + const call = async () => { + calls++; + return { + data: null, + error: { + message: 'Internal server error', + name: 'internal_server_error', + }, + headers: null, + }; + }; + + let threw = false; + try { + await withSpinner( + { loading: 'Loading...' }, + call, + 'test_error', + globalOpts, + ); + } catch (err) { + threw = true; + expect(err).toBeInstanceOf(ExitError); + } + expect(threw).toBe(true); + expect(calls).toBe(1); + }); + + it('exhausts transient retries and errors after max attempts', async () => { + let calls = 0; + const call = async () => { + calls++; + return { + data: null, + error: { + message: 'Internal server error', + name: 'internal_server_error', + }, + headers: { 'retry-after': '0' }, + }; + }; + + let threw = false; + try { + await withSpinner( + { loading: 'Loading...', retryTransient: true }, + call, + 'test_error', + globalOpts, + ); + } catch (err) { + threw = true; + expect(err).toBeInstanceOf(ExitError); + } + expect(threw).toBe(true); + expect(calls).toBe(4); + }); + + it('still retries rate_limit_exceeded even without retryTransient', async () => { + let calls = 0; + const call = async () => { + calls++; + if (calls === 1) { + return { + data: null, + error: { + message: 'Rate limit exceeded', + name: 'rate_limit_exceeded', + }, + headers: { 'retry-after': '0' }, + }; + } + return { data: { id: 'ok' }, error: null, headers: null }; + }; + + const result = await withSpinner( + { loading: 'Loading...' }, + call, + 'test_error', + globalOpts, + ); + expect(result).toEqual({ id: 'ok' }); + expect(calls).toBe(2); + }); +}); + describe('createSpinner', () => { const originalStdinIsTTY = process.stdin.isTTY; const originalStdoutIsTTY = process.stdout.isTTY; @@ -224,7 +409,6 @@ describe('createSpinner', () => { const { createSpinner } = await import('../../src/lib/spinner'); const spinner = createSpinner('test message'); - // Should not throw when calling any method expect(() => spinner.stop('done')).not.toThrow(); expect(() => spinner.fail('error')).not.toThrow(); expect(() => spinner.warn('warning')).not.toThrow(); @@ -256,7 +440,6 @@ describe('createSpinner', () => { expect(spinner).toHaveProperty('warn'); expect(spinner).toHaveProperty('update'); - // Stop to clean up the interval spinner.stop('done'); expect(stderrSpy).toHaveBeenCalled(); });