From 6d98a55980728d330c4d977e998288903df0ad5e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 9 Apr 2026 17:35:57 +0000 Subject: [PATCH 1/3] fix: add 30s request timeout to withSpinner to prevent hanging CLI commands Adds a withTimeout utility that wraps SDK call promises with a 30-second deadline. When the timeout expires, the promise rejects with a structured error that flows through the existing error handling in withSpinner. This prevents contact commands (and all other SDK-backed commands) from hanging indefinitely when the upstream API is slow or unresponsive. Resolves BU-640 Co-authored-by: Bu Kinoshita --- src/lib/spinner.ts | 6 +++++- src/lib/with-timeout.ts | 21 +++++++++++++++++++++ tests/lib/spinner.test.ts | 26 ++++++++++++++++++++++++++ tests/lib/with-timeout.test.ts | 23 +++++++++++++++++++++++ 4 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 src/lib/with-timeout.ts create mode 100644 tests/lib/with-timeout.test.ts diff --git a/src/lib/spinner.ts b/src/lib/spinner.ts index 88e4134a..1d4edcf7 100644 --- a/src/lib/spinner.ts +++ b/src/lib/spinner.ts @@ -2,6 +2,7 @@ import pc from 'picocolors'; import type { GlobalOpts } from './client'; import { errorMessage, outputError } from './output'; import { isInteractive, isUnicodeSupported } from './tty'; +import { REQUEST_TIMEOUT_MS, withTimeout } from './with-timeout'; // Status symbols generated via String.fromCodePoint() — never literal Unicode in // source — to prevent UTF-8 → Latin-1 corruption when the npm package is bundled. @@ -67,7 +68,10 @@ export async function withSpinner( const spinner = createSpinner(loading, globalOpts.quiet); try { for (let attempt = 0; ; attempt++) { - const { data, error, headers } = await call(); + const { data, error, headers } = await withTimeout( + call(), + REQUEST_TIMEOUT_MS, + ); if (error) { if (attempt < MAX_RETRIES && error.name === 'rate_limit_exceeded') { const delay = diff --git a/src/lib/with-timeout.ts b/src/lib/with-timeout.ts new file mode 100644 index 00000000..39942093 --- /dev/null +++ b/src/lib/with-timeout.ts @@ -0,0 +1,21 @@ +const REQUEST_TIMEOUT_MS = 30_000; + +const withTimeout = (promise: Promise, ms: number): Promise => + new Promise((resolve, reject) => { + const timer = setTimeout(() => { + reject(new Error(`Request timed out after ${ms / 1000}s`)); + }, ms); + timer.unref(); + promise.then( + (val) => { + clearTimeout(timer); + resolve(val); + }, + (err) => { + clearTimeout(timer); + reject(err); + }, + ); + }); + +export { REQUEST_TIMEOUT_MS, withTimeout }; diff --git a/tests/lib/spinner.test.ts b/tests/lib/spinner.test.ts index 395c6308..427ff546 100644 --- a/tests/lib/spinner.test.ts +++ b/tests/lib/spinner.test.ts @@ -8,6 +8,7 @@ import { vi, } from 'vitest'; import { withSpinner } from '../../src/lib/spinner'; +import * as timeoutModule from '../../src/lib/with-timeout'; import { captureTestEnv, ExitError, @@ -187,6 +188,31 @@ describe('withSpinner retry on rate_limit_exceeded', () => { // Default first retry delay is 1s expect(Date.now() - start).toBeGreaterThanOrEqual(900); }); + + test('exits with error when request times out', async () => { + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + vi.spyOn(timeoutModule, 'withTimeout').mockRejectedValue( + new Error('Request timed out after 30s'), + ); + + let threw = false; + try { + await withSpinner( + msgs, + async () => ({ data: null, error: null, headers: null }), + 'test_error', + globalOpts, + ); + } catch (err) { + threw = true; + expect(err).toBeInstanceOf(ExitError); + expect((err as ExitError).code).toBe(1); + } + expect(threw).toBe(true); + const logOutput = logSpy.mock.calls.flat().join(' '); + expect(logOutput).toContain('timed out'); + logSpy.mockRestore(); + }); }); describe('createSpinner', () => { diff --git a/tests/lib/with-timeout.test.ts b/tests/lib/with-timeout.test.ts new file mode 100644 index 00000000..125dcbd6 --- /dev/null +++ b/tests/lib/with-timeout.test.ts @@ -0,0 +1,23 @@ +import { describe, expect, it } from 'vitest'; +import { withTimeout } from '../../src/lib/with-timeout'; + +describe('withTimeout', () => { + it('resolves when the promise completes before the deadline', async () => { + const result = await withTimeout(Promise.resolve(42), 1000); + expect(result).toBe(42); + }); + + it('rejects when the promise exceeds the deadline', async () => { + const slow = new Promise((resolve) => + setTimeout(() => resolve('late'), 5000), + ); + await expect(withTimeout(slow, 50)).rejects.toThrow( + 'Request timed out after 0.05s', + ); + }); + + it('forwards the original rejection when the promise fails before the deadline', async () => { + const failing = Promise.reject(new Error('boom')); + await expect(withTimeout(failing, 1000)).rejects.toThrow('boom'); + }); +}); From 60eff6c7548529926c014467fa361dcce5bbc0ba Mon Sep 17 00:00:00 2001 From: Bu Kinoshita Date: Mon, 13 Apr 2026 21:50:10 +0000 Subject: [PATCH 2/3] fix: test --- tests/lib/spinner.test.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/lib/spinner.test.ts b/tests/lib/spinner.test.ts index 427ff546..cb6c372c 100644 --- a/tests/lib/spinner.test.ts +++ b/tests/lib/spinner.test.ts @@ -190,7 +190,6 @@ describe('withSpinner retry on rate_limit_exceeded', () => { }); test('exits with error when request times out', async () => { - const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); vi.spyOn(timeoutModule, 'withTimeout').mockRejectedValue( new Error('Request timed out after 30s'), ); @@ -209,9 +208,8 @@ describe('withSpinner retry on rate_limit_exceeded', () => { expect((err as ExitError).code).toBe(1); } expect(threw).toBe(true); - const logOutput = logSpy.mock.calls.flat().join(' '); - expect(logOutput).toContain('timed out'); - logSpy.mockRestore(); + const errOutput = errorSpy.mock.calls.flat().join(' '); + expect(errOutput).toContain('timed out'); }); }); From 4403b4957aa3b303e417f6039b27656bf9b9dc10 Mon Sep 17 00:00:00 2001 From: Felipe Freitag Date: Thu, 16 Apr 2026 17:45:47 -0300 Subject: [PATCH 3/3] fix: use `it` instead of `test` in spinner timeout test `test` is not imported from vitest and globals are not enabled, causing a ReferenceError that crashes the entire spinner test file. --- tests/lib/spinner.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lib/spinner.test.ts b/tests/lib/spinner.test.ts index c1197583..8895c14e 100644 --- a/tests/lib/spinner.test.ts +++ b/tests/lib/spinner.test.ts @@ -189,7 +189,7 @@ describe('withSpinner retry on rate_limit_exceeded', () => { expect(Date.now() - start).toBeGreaterThanOrEqual(900); }); - test('exits with error when request times out', async () => { + it('exits with error when request times out', async () => { vi.spyOn(timeoutModule, 'withTimeout').mockRejectedValue( new Error('Request timed out after 30s'), );