diff --git a/apps/sim/executor/handlers/agent/agent-handler.test.ts b/apps/sim/executor/handlers/agent/agent-handler.test.ts index 0da21a6a431..35dbf52d7f3 100644 --- a/apps/sim/executor/handlers/agent/agent-handler.test.ts +++ b/apps/sim/executor/handlers/agent/agent-handler.test.ts @@ -979,6 +979,49 @@ describe('AgentBlockHandler', () => { ) }) + /** + * A stalled model call reaches here as the runtime's own `TimeoutError`, whose bare + * message ("The operation timed out.") names nothing. It must become a Sim-level + * message WITHOUT discarding the phase detail the provider attached — that detail is + * the only thing distinguishing "never answered" from "body never completed". + */ + it('maps a provider TimeoutError to a Sim message while keeping the phase detail', async () => { + const inputs = { model: 'gpt-4o', userPrompt: 'hi', apiKey: 'test-api-key' } + mockGetProviderFromModel.mockReturnValue('openai') + + // Faithful to production: providers rewrap the transport failure in a + // ProviderError, which overwrites `name` — so only the cause still classifies it. + const transport = new Error( + 'The operation timed out. [phase=reading-response-body elapsedMs=60001 status=200 contentLength=32116]' + ) + transport.name = 'TimeoutError' + const wrapped = new Error(transport.message, { cause: transport }) + wrapped.name = 'ProviderError' + mockExecuteProviderRequest.mockRejectedValueOnce(wrapped) + + const error = await handler.execute(mockContext, mockBlock, inputs).catch((e) => e) + + expect(error.message).toContain('Provider request timed out') + expect(error.message).toContain('phase=reading-response-body') + expect(error.message).toContain('status=200') + }) + + it('maps a provider AbortError the same way', async () => { + const inputs = { model: 'gpt-4o', userPrompt: 'hi', apiKey: 'test-api-key' } + mockGetProviderFromModel.mockReturnValue('openai') + + const aborted = new Error('aborted [phase=awaiting-response-headers elapsedMs=12]') + aborted.name = 'AbortError' + const wrapped = new Error(aborted.message, { cause: aborted }) + wrapped.name = 'ProviderError' + mockExecuteProviderRequest.mockRejectedValueOnce(wrapped) + + const error = await handler.execute(mockContext, mockBlock, inputs).catch((e) => e) + + expect(error.message).toContain('Provider request timed out') + expect(error.message).toContain('phase=awaiting-response-headers') + }) + it('should handle streaming responses with text/event-stream content type', async () => { const mockStreamBody = new ReadableStream({ start(controller) { diff --git a/apps/sim/executor/handlers/agent/agent-handler.ts b/apps/sim/executor/handlers/agent/agent-handler.ts index 8d510d1696e..54d8adffe33 100644 --- a/apps/sim/executor/handlers/agent/agent-handler.ts +++ b/apps/sim/executor/handlers/agent/agent-handler.ts @@ -71,6 +71,22 @@ import { getToolAsync } from '@/tools/utils.server' const logger = createLogger('AgentBlockHandler') +/** + * True when a failure originated from a transport deadline or abort, at any depth of the + * cause chain. + * + * Providers rewrap transport failures (`ProviderError` overwrites `name`), so a check on + * the top-level `name` alone misses every wrapped case. Bounded to a short walk so a + * self-referential cause cannot loop. + */ +function isTransportTimeout(error: unknown): boolean { + for (let current = error, depth = 0; current instanceof Error && depth < 5; depth++) { + if (current.name === 'AbortError' || current.name === 'TimeoutError') return true + current = current.cause + } + return false +} + /** * Handler for Agent blocks that process LLM requests with optional tools. */ @@ -1299,8 +1315,20 @@ export class AgentBlockHandler implements BlockHandler { timestamp: new Date().toISOString(), }) - if (error.name === 'AbortError') { - throw new Error('Provider request timed out - the API took too long to respond') + /** + * `TimeoutError` is what the runtime raises on a fetch deadline; without it a + * stalled model call reached the trace as the bare runtime string. + * + * The cause chain is walked, not just `name`: providers rewrap transport failures in + * a `ProviderError`, which overwrites `name`, so the classification only survives on + * `cause`. The original message is kept rather than replaced — providers annotate it + * with the request phase they died in, and that detail is the only thing separating a + * request that was never answered from one whose body stalled. + */ + if (isTransportTimeout(error)) { + throw new Error( + `Provider request timed out - the API took too long to respond (${error.message})` + ) } if (error.name === 'TypeError' && error.message.includes('fetch')) { throw new Error( diff --git a/apps/sim/providers/openai/core.response-status.test.ts b/apps/sim/providers/openai/core.response-status.test.ts new file mode 100644 index 00000000000..e12df1fa42a --- /dev/null +++ b/apps/sim/providers/openai/core.response-status.test.ts @@ -0,0 +1,258 @@ +/** + * @vitest-environment node + * + * `/v1/responses` answers HTTP 200 for generations that did not succeed — `status: + * 'failed'` with a populated `error`, or `status: 'incomplete'` with a reason. The + * non-streaming path read only `output`, so those reached the user as a success with + * empty content and billed tokens, while the trace span independently recorded + * `finishReason: 'error'`. + * + * These cover the status/error gate and pin the `incomplete` policy to the one the + * streaming loop already applies, so the two paths cannot silently diverge again. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { executeResponsesProviderRequest } from '@/providers/openai/core' +import type { ProviderRequest, ProviderResponse } from '@/providers/types' + +vi.mock('@/providers', () => ({ MAX_TOOL_ITERATIONS: 5 })) + +vi.mock('@/providers/utils', () => ({ + isFunctionToolCall: () => false, + calculateCost: () => ({ input: 0, output: 0, total: 0 }), + sumToolCosts: () => 0, + enforceStrictSchema: (schema: unknown) => schema, + prepareToolExecution: () => ({ toolParams: {}, executionParams: {} }), + prepareToolsWithUsageControl: (tools: unknown[]) => ({ + tools, + toolChoice: undefined, + forcedTools: [], + hasFilteredTools: false, + }), + trackForcedToolUsage: () => ({ hasUsedForcedTool: false, usedForcedTools: [] }), + supportsReasoningEffort: () => false, +})) + +const { mockExecuteProviderTool } = vi.hoisted(() => ({ + mockExecuteProviderTool: vi.fn(), +})) + +vi.mock('@/providers/runtime-context', () => ({ + executeProviderTool: mockExecuteProviderTool, +})) + +function jsonResponse(body: unknown) { + return { + ok: true, + status: 200, + headers: new Headers(), + json: () => Promise.resolve(body), + } +} + +const USAGE = { input_tokens: 1, output_tokens: 1, total_tokens: 2 } + +function message(text: string) { + return { + type: 'message', + role: 'assistant', + content: [{ type: 'output_text', text }], + } +} + +function functionCall(args: string) { + return { type: 'function_call', call_id: 'call_1', name: 'exa_search', arguments: args } +} + +const COMPLETED_RESPONSE = { + id: 'resp_1', + status: 'completed', + error: null, + incomplete_details: null, + output: [message('hello')], + usage: USAGE, +} + +describe('OpenAI non-streaming response status handling', () => { + const logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() } as any + + beforeEach(() => { + vi.clearAllMocks() + mockExecuteProviderTool.mockResolvedValue({ success: true, output: { results: [] } }) + }) + + function run(fetchMock: unknown, request: Partial = {}) { + return executeResponsesProviderRequest( + { apiKey: 'k', model: 'gpt-5.5', messages: [{ role: 'user', content: 'hi' }], ...request }, + { + providerId: 'openai', + providerLabel: 'OpenAI', + modelName: 'gpt-5.5', + endpoint: 'https://api.openai.com/v1/responses', + headers: { Authorization: 'Bearer k' }, + logger, + fetch: fetchMock as typeof fetch, + } + ) + } + + const TOOL_REQUEST: Partial = { + tools: [{ id: 'exa_search', name: 'exa_search', description: 'search', params: {} }], + } + + it('fails the block on a 200 carrying status "failed", surfacing the API error message', async () => { + const fetchMock = vi.fn().mockResolvedValue( + jsonResponse({ + id: 'resp_1', + status: 'failed', + error: { code: 'server_error', message: 'The model produced an invalid response.' }, + incomplete_details: null, + output: [], + usage: USAGE, + }) + ) + + await expect(run(fetchMock)).rejects.toThrow('The model produced an invalid response.') + }) + + it('fails the block when error is populated but status is absent', async () => { + const fetchMock = vi.fn().mockResolvedValue( + jsonResponse({ + id: 'resp_1', + error: { code: null, message: 'Upstream provider rejected the request.' }, + incomplete_details: null, + output: [], + usage: USAGE, + }) + ) + + await expect(run(fetchMock)).rejects.toThrow('Upstream provider rejected the request.') + }) + + /** + * Decision, matching `streamResponsesTurn`: an `incomplete` response truncated by + * `max_output_tokens` with no tool call is NOT an error — the partial prose is a + * usable answer and is returned as the block content. + */ + it('returns the partial content of a max_output_tokens incomplete response instead of failing', async () => { + const fetchMock = vi.fn().mockResolvedValue( + jsonResponse({ + id: 'resp_1', + status: 'incomplete', + error: null, + incomplete_details: { reason: 'max_output_tokens' }, + output: [message('a truncated but usable answer')], + usage: USAGE, + }) + ) + + const result = (await run(fetchMock)) as ProviderResponse + expect(result.content).toBe('a truncated but usable answer') + }) + + /** + * The other half of the same decision: every other incomplete reason is an error, + * because the generation stopped for a reason the caller must be told about. + */ + it('fails the block on an incomplete response whose reason is not max_output_tokens', async () => { + const fetchMock = vi.fn().mockResolvedValue( + jsonResponse({ + id: 'resp_1', + status: 'incomplete', + error: null, + incomplete_details: { reason: 'content_filter' }, + output: [message('partial')], + usage: USAGE, + }) + ) + + await expect(run(fetchMock)).rejects.toThrow(/content_filter/) + }) + + /** + * The confusing-failure case: a truncated `function_call` holds half-written JSON. + * Executing it made `parseToolArguments` throw, reporting a tool bug rather than the + * truncation that actually happened. + */ + it('does not execute a tool call from a non-completed response', async () => { + const fetchMock = vi.fn().mockResolvedValue( + jsonResponse({ + id: 'resp_1', + status: 'incomplete', + error: null, + incomplete_details: { reason: 'max_output_tokens' }, + output: [functionCall('{"query": "half writ')], + usage: USAGE, + }) + ) + + await expect(run(fetchMock, TOOL_REQUEST)).rejects.toThrow(/max_output_tokens/) + expect(mockExecuteProviderTool).not.toHaveBeenCalled() + }) + + it('leaves a healthy completed response entirely unaffected', async () => { + const fetchMock = vi.fn().mockResolvedValue(jsonResponse(COMPLETED_RESPONSE)) + + const result = (await run(fetchMock)) as ProviderResponse + expect(result.content).toBe('hello') + expect(result.toolCalls).toBeUndefined() + expect(result.tokens?.total).toBe(2) + expect(fetchMock).toHaveBeenCalledTimes(1) + }) + + it('still runs the multi-turn tool loop end to end', async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce( + jsonResponse({ + id: 'resp_tool', + status: 'completed', + error: null, + incomplete_details: null, + output: [functionCall('{"query":"sim"}')], + usage: USAGE, + }) + ) + .mockResolvedValueOnce(jsonResponse(COMPLETED_RESPONSE)) + + const result = (await run(fetchMock, TOOL_REQUEST)) as ProviderResponse + + expect(fetchMock).toHaveBeenCalledTimes(2) + expect(mockExecuteProviderTool).toHaveBeenCalledTimes(1) + expect(result.toolCalls).toHaveLength(1) + expect(result.toolCalls?.[0].success).toBe(true) + expect(result.content).toBe('hello') + expect(result.tokens?.total).toBe(4) + }) + + /** + * The gate sits in `postResponses`, so it must cover continuation turns too — a loop + * that starts healthy and fails on turn two must still fail the block. + */ + it('fails the block when a later tool-loop turn comes back failed', async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce( + jsonResponse({ + id: 'resp_tool', + status: 'completed', + error: null, + incomplete_details: null, + output: [functionCall('{"query":"sim"}')], + usage: USAGE, + }) + ) + .mockResolvedValueOnce( + jsonResponse({ + id: 'resp_2', + status: 'failed', + error: { code: 'server_error', message: 'Second turn blew up.' }, + incomplete_details: null, + output: [], + usage: USAGE, + }) + ) + + await expect(run(fetchMock, TOOL_REQUEST)).rejects.toThrow('Second turn blew up.') + expect(mockExecuteProviderTool).toHaveBeenCalledTimes(1) + }) +}) diff --git a/apps/sim/providers/openai/core.retry.test.ts b/apps/sim/providers/openai/core.retry.test.ts new file mode 100644 index 00000000000..91c66333ae0 --- /dev/null +++ b/apps/sim/providers/openai/core.retry.test.ts @@ -0,0 +1,314 @@ +/** + * @vitest-environment node + * + * `/v1/responses` is posted with raw `fetch`, which dropped the OpenAI SDK's own + * `maxRetries: 2` when this path moved off the SDK. These cover the restored + * status-based retries — and, just as importantly, the classes that must stay + * non-retryable: a caller abort, and a stalled body, which arrives only after a + * response already exists and would therefore be billed twice. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { executeResponsesProviderRequest } from '@/providers/openai/core' +import type { ProviderRequest } from '@/providers/types' + +vi.mock('@/providers', () => ({ MAX_TOOL_ITERATIONS: 5 })) + +vi.mock('@/providers/utils', () => ({ + isFunctionToolCall: () => false, + calculateCost: () => ({ input: 0, output: 0, total: 0 }), + sumToolCosts: () => 0, + enforceStrictSchema: (schema: unknown) => schema, + prepareToolExecution: () => ({ toolParams: {}, executionParams: {} }), + prepareToolsWithUsageControl: (tools: unknown[]) => ({ + tools, + toolChoice: undefined, + forcedTools: [], + hasFilteredTools: false, + }), + trackForcedToolUsage: () => ({ hasUsedForcedTool: false, usedForcedTools: [] }), + supportsReasoningEffort: () => false, +})) + +vi.mock('@/tools', () => ({ executeTool: vi.fn() })) + +const COMPLETED_RESPONSE = { + id: 'resp_1', + status: 'completed', + output: [{ type: 'message', role: 'assistant', content: [{ type: 'output_text', text: 'ok' }] }], + usage: { input_tokens: 1, output_tokens: 1, total_tokens: 2 }, +} + +function okResponse() { + return { + ok: true, + status: 200, + headers: new Headers(), + json: () => Promise.resolve(COMPLETED_RESPONSE), + } +} + +function errorResponse(status: number, headers: Record = {}) { + return { + ok: false, + status, + headers: new Headers(headers), + text: () => Promise.resolve(JSON.stringify({ error: { message: `boom ${status}` } })), + } +} + +/** + * Exactly what the runtime raises when a fetch deadline fires: a `DOMException`, + * NOT a plain `Error`. `DOMException.message` is a readonly getter, so a plain + * `Error` here would not exercise the real failure shape. + */ +function timeoutError() { + return new DOMException('The operation timed out.', 'TimeoutError') +} + +/** A 200 whose body never settles until the request's signal aborts. */ +function stallingBody(init: RequestInit) { + return { + ok: true, + status: 200, + headers: new Headers(), + json: () => + new Promise((_resolve, reject) => { + if (init.signal?.aborted) { + reject(timeoutError()) + return + } + init.signal?.addEventListener('abort', () => reject(timeoutError()), { once: true }) + }), + } +} + +describe('OpenAI Responses status retries', () => { + const logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() } as never + + beforeEach(() => vi.clearAllMocks()) + + function run(fetchMock: unknown, request: Partial = {}) { + return executeResponsesProviderRequest( + { + apiKey: 'k', + model: 'gpt-5.5', + messages: [{ role: 'user', content: 'hi' }], + workflowId: 'wf_1', + blockId: 'blk_1', + executionId: 'exec_1', + ...request, + }, + { + providerId: 'openai', + providerLabel: 'OpenAI', + modelName: 'gpt-5.5', + endpoint: 'https://api.openai.com/v1/responses', + headers: { Authorization: 'Bearer k' }, + logger, + fetch: fetchMock as typeof fetch, + } + ) + } + + /** + * Drives a run to completion under fake timers so backoff costs no wall time. + * + * Timers are restored on a single exit path rather than in a `finally` after a + * `return`: biome reads `vi.useRealTimers` as a React hook and rejects that shape + * as a conditionally-called hook. + */ + async function runWithTimers(fetchMock: unknown, request: Partial = {}) { + vi.useFakeTimers() + const promise = run(fetchMock, request).catch((error: unknown) => error) + await vi.advanceTimersByTimeAsync(120_000) + const settled = await promise + vi.useRealTimers() + return settled + } + + it('retries a 429 and then succeeds', async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(errorResponse(429)) + .mockResolvedValueOnce(okResponse()) + + const result = await runWithTimers(fetchMock) + + expect(fetchMock).toHaveBeenCalledTimes(2) + expect(result).toMatchObject({ content: 'ok' }) + }) + + it('retries a 500 and then succeeds', async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(errorResponse(500)) + .mockResolvedValueOnce(okResponse()) + + const result = await runWithTimers(fetchMock) + + expect(fetchMock).toHaveBeenCalledTimes(2) + expect(result).toMatchObject({ content: 'ok' }) + }) + + it('logs each retry with the attempt, status, delay and correlation ids', async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(errorResponse(503)) + .mockResolvedValueOnce(okResponse()) + + await runWithTimers(fetchMock) + + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('retryable status'), + expect.objectContaining({ + attempt: 1, + status: 503, + delayMs: expect.any(Number), + workflowId: 'wf_1', + blockId: 'blk_1', + executionId: 'exec_1', + }) + ) + }) + + it('does not retry a 400', async () => { + const fetchMock = vi.fn().mockResolvedValue(errorResponse(400)) + + const error = await runWithTimers(fetchMock) + + expect(fetchMock).toHaveBeenCalledTimes(1) + expect((error as Error).message).toContain('boom 400') + }) + + it('does not retry a 401', async () => { + const fetchMock = vi.fn().mockResolvedValue(errorResponse(401)) + + const error = await runWithTimers(fetchMock) + + expect(fetchMock).toHaveBeenCalledTimes(1) + expect((error as Error).message).toContain('boom 401') + }) + + it('gives up after the maximum attempts and surfaces the final error', async () => { + const fetchMock = vi.fn().mockResolvedValue(errorResponse(429)) + + const error = await runWithTimers(fetchMock) + + expect(fetchMock).toHaveBeenCalledTimes(3) + expect((error as Error).message).toContain('OpenAI API error (429): boom 429') + }) + + it('honours Retry-After before re-sending', async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(errorResponse(429, { 'retry-after': '5' })) + .mockResolvedValueOnce(okResponse()) + + vi.useFakeTimers() + try { + const promise = run(fetchMock).catch((error: unknown) => error) + + await vi.advanceTimersByTimeAsync(4_000) + expect(fetchMock).toHaveBeenCalledTimes(1) + + await vi.advanceTimersByTimeAsync(1_500) + expect(fetchMock).toHaveBeenCalledTimes(2) + + await vi.advanceTimersByTimeAsync(1_000) + await promise + } finally { + vi.useRealTimers() + } + }) + + it('prefers retry-after-ms over Retry-After', async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(errorResponse(429, { 'retry-after-ms': '8000', 'retry-after': '1' })) + .mockResolvedValueOnce(okResponse()) + + vi.useFakeTimers() + try { + const promise = run(fetchMock).catch((error: unknown) => error) + + await vi.advanceTimersByTimeAsync(7_000) + expect(fetchMock).toHaveBeenCalledTimes(1) + + await vi.advanceTimersByTimeAsync(1_500) + expect(fetchMock).toHaveBeenCalledTimes(2) + + await vi.advanceTimersByTimeAsync(1_000) + await promise + } finally { + vi.useRealTimers() + } + }) + + it('bounds a stalled error body instead of hanging on it', async () => { + // Non-2xx headers, then an error body that never settles. + const fetchMock = vi.fn().mockImplementation((_url: string, init: RequestInit) => ({ + ok: false, + status: 400, + headers: new Headers(), + text: () => + new Promise((_resolve, reject) => { + if (init.signal?.aborted) { + reject(new DOMException('The operation timed out.', 'TimeoutError')) + return + } + init.signal?.addEventListener( + 'abort', + () => reject(new DOMException('The operation timed out.', 'TimeoutError')), + { once: true } + ) + }), + })) + + const settled = await runWithTimers(fetchMock) + + expect(settled).toBeInstanceOf(Error) + expect(fetchMock).toHaveBeenCalledTimes(1) + }) + + it('abandons backoff immediately when the caller aborts, and reports the abort', async () => { + const caller = new AbortController() + const fetchMock = vi.fn().mockImplementation(() => { + queueMicrotask(() => caller.abort(new DOMException('timeout', 'AbortError'))) + return errorResponse(429) + }) + + const settled = (await runWithTimers(fetchMock, { abortSignal: caller.signal })) as Error + + // The cancellation must surface as the abort, not as the stale 429. + expect(settled.message).not.toContain('429') + expect(fetchMock).toHaveBeenCalledTimes(1) + }) + + it('does not retry when the caller aborts', async () => { + const caller = new AbortController() + const fetchMock = vi.fn().mockImplementation(() => { + caller.abort(new DOMException('workflow cancelled', 'AbortError')) + return Promise.resolve(errorResponse(429)) + }) + + await runWithTimers(fetchMock, { abortSignal: caller.signal }) + + expect(fetchMock).toHaveBeenCalledTimes(1) + }) + + /** + * The double-billing guard. A stalled body means the response was created and + * billed server-side; `/v1/responses` ignores `Idempotency-Key`, so re-sending + * would generate and bill a second one. + */ + it('does not retry a body stall', async () => { + const fetchMock = vi + .fn() + .mockImplementation((_url: string, init: RequestInit) => Promise.resolve(stallingBody(init))) + + const error = await runWithTimers(fetchMock) + + expect(fetchMock).toHaveBeenCalledTimes(1) + expect((error as Error).message).toContain('phase=reading-response-body') + }) +}) diff --git a/apps/sim/providers/openai/core.transport-phase.test.ts b/apps/sim/providers/openai/core.transport-phase.test.ts new file mode 100644 index 00000000000..086863d44a7 --- /dev/null +++ b/apps/sim/providers/openai/core.transport-phase.test.ts @@ -0,0 +1,288 @@ +/** + * @vitest-environment node + * + * A stalled model call surfaces only the runtime's own `TimeoutError: The + * operation timed out.`, which cannot distinguish "never answered" from + * "answered but the body never completed" — opposite causes with opposite fixes. + * These cover the phase annotation that makes the distinction observable from the + * execution trace, which survives when a task stops shipping logs. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { executeResponsesProviderRequest } from '@/providers/openai/core' +import type { ProviderRequest } from '@/providers/types' + +vi.mock('@/providers', () => ({ MAX_TOOL_ITERATIONS: 5 })) + +vi.mock('@/providers/utils', () => ({ + isFunctionToolCall: () => false, + calculateCost: () => ({ input: 0, output: 0, total: 0 }), + sumToolCosts: () => 0, + enforceStrictSchema: (schema: unknown) => schema, + prepareToolExecution: () => ({ toolParams: {}, executionParams: {} }), + prepareToolsWithUsageControl: (tools: unknown[]) => ({ + tools, + toolChoice: undefined, + forcedTools: [], + hasFilteredTools: false, + }), + trackForcedToolUsage: () => ({ hasUsedForcedTool: false, usedForcedTools: [] }), + supportsReasoningEffort: () => false, +})) + +vi.mock('@/tools', () => ({ executeTool: vi.fn() })) + +/** + * Exactly what the runtime raises when a fetch deadline fires: a `DOMException`, + * NOT a plain `Error`. The distinction is load-bearing — `DOMException.message` is a + * readonly getter, so annotating by assignment throws a TypeError and replaces the + * real failure. Constructing a plain Error here would let that regression pass. + */ +function timeoutError() { + return new DOMException('The operation timed out.', 'TimeoutError') +} + +/** + * A response whose body never settles until the request's signal aborts — the shape of + * the `/v1/responses` stall. Rejects immediately if the signal already aborted, so the + * body can never outlive an abort that landed before the listener attached. + */ +function stallingBody(init: RequestInit, responseInit: Partial = {}) { + return { + ok: true, + status: 200, + headers: new Headers(), + ...responseInit, + json: () => + new Promise((_resolve, reject) => { + if (init.signal?.aborted) { + reject(timeoutError()) + return + } + init.signal?.addEventListener('abort', () => reject(timeoutError()), { once: true }) + }), + } +} + +describe('OpenAI transport phase annotation', () => { + const logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() } as any + + beforeEach(() => vi.clearAllMocks()) + + function run(fetchMock: unknown, request: Partial = {}) { + return executeResponsesProviderRequest( + { apiKey: 'k', model: 'gpt-5.5', messages: [{ role: 'user', content: 'hi' }], ...request }, + { + providerId: 'openai', + providerLabel: 'OpenAI', + modelName: 'gpt-5.5', + endpoint: 'https://api.openai.com/v1/responses', + headers: { Authorization: 'Bearer k' }, + logger, + fetch: fetchMock as typeof fetch, + } + ) + } + + it('names the body phase, with response metadata, when headers arrived but the body stalled', async () => { + const stalledBody = { + ok: true, + status: 200, + headers: new Headers({ 'content-length': '32116', 'content-encoding': 'br' }), + json: () => Promise.reject(timeoutError()), + } + + await expect(run(vi.fn().mockResolvedValue(stalledBody))).rejects.toThrow( + /phase=reading-response-body/ + ) + }) + + it('carries the status, content-length and content-encoding of the stalled response', async () => { + const stalledBody = { + ok: true, + status: 200, + headers: new Headers({ 'content-length': '32116', 'content-encoding': 'br' }), + json: () => Promise.reject(timeoutError()), + } + + const error = await run(vi.fn().mockResolvedValue(stalledBody)).catch((e) => e) + expect(error.message).toContain('status=200') + expect(error.message).toContain('contentLength=32116') + expect(error.message).toContain('contentEncoding=br') + expect(error.message).toMatch(/ttfbMs=\d+/) + }) + + /** + * `x-request-id` is the only identifier OpenAI support can trace a call by, and a + * stalled request is precisely when we need to hand them one. + */ + it('carries the OpenAI x-request-id of the stalled response', async () => { + const stalledBody = { + ok: true, + status: 200, + headers: new Headers({ 'x-request-id': 'req_abc123', 'content-length': '32116' }), + json: () => Promise.reject(timeoutError()), + } + + const error = await run(vi.fn().mockResolvedValue(stalledBody)).catch((e) => e) + expect(error.message).toContain('requestId=req_abc123') + }) + + it('bounds a non-JSON error body instead of pasting a gateway page into the error', async () => { + const htmlError = { + ok: false, + status: 502, + headers: new Headers(), + text: () => Promise.resolve(`${'x'.repeat(5000)}`), + } + + const error = await run(vi.fn().mockResolvedValue(htmlError)).catch((e) => e) + expect(error.message.length).toBeLessThan(700) + }) + + it('names the header phase when nothing came back at all', async () => { + const error = await run(vi.fn().mockRejectedValue(timeoutError())).catch((e) => e) + expect(error.message).toContain('phase=awaiting-response-headers') + // No response existed, so no response metadata may be claimed. + expect(error.message).not.toContain('status=') + }) + + it('does not retry a stalled body — the endpoint ignores Idempotency-Key, so a retry would double-create', async () => { + const fetchMock = vi + .fn() + .mockImplementation((_url: string, init: RequestInit) => Promise.resolve(stallingBody(init))) + + vi.useFakeTimers() + try { + const promise = run(fetchMock).catch((e) => e) + await vi.advanceTimersByTimeAsync(120_000) + await promise + } finally { + vi.useRealTimers() + } + + expect(fetchMock).toHaveBeenCalledTimes(1) + const sent = fetchMock.mock.calls[0][1].headers as Record + expect(sent['Idempotency-Key']).toBeUndefined() + }) + + it('bounds a stalled body instead of waiting for the runtime socket wall', async () => { + const fetchMock = vi + .fn() + .mockImplementation((_url: string, init: RequestInit) => Promise.resolve(stallingBody(init))) + + vi.useFakeTimers() + let error: any + try { + const promise = run(fetchMock).catch((e) => e) + await vi.advanceTimersByTimeAsync(120_000) + error = await promise + } finally { + vi.useRealTimers() + } + + expect(error.message).toContain('phase=reading-response-body') + }) + + it('leaves a self-describing API error untouched', async () => { + const apiError = { + ok: false, + status: 429, + headers: new Headers(), + text: () => Promise.resolve(JSON.stringify({ error: { message: 'Rate limit reached' } })), + } + + const error = await run(vi.fn().mockResolvedValue(apiError)).catch((e) => e) + expect(error.message).toContain('Rate limit reached') + expect(error.message).not.toContain('phase=') + }) + + /** + * The load-bearing design decision. `/v1/responses` withholds its 200 until generation + * has finished, so all think time is time-to-headers — measured at 14545ms to headers + * and 1ms of body on a real long call. Bounding headers would therefore fail healthy + * reasoning runs. If someone later "tidies" the deadline to cover the whole request, + * this test is what stops it. + */ + it('does not bound time-to-headers, however long generation takes', async () => { + const completed = { + id: 'resp_1', + status: 'completed', + output: [ + { type: 'message', role: 'assistant', content: [{ type: 'output_text', text: 'ok' }] }, + ], + usage: { input_tokens: 1, output_tokens: 1, total_tokens: 2 }, + } + /** + * Headers arrive only after far longer than the body budget. The mock honours the + * signal the way a real fetch does, so a deadline armed before headers would reject + * here — that is what makes this test able to fail. + */ + const fetchMock = vi.fn().mockImplementation( + (_url: string, init: RequestInit) => + new Promise((resolve, reject) => { + if (init.signal?.aborted) { + reject(timeoutError()) + return + } + init.signal?.addEventListener('abort', () => reject(timeoutError()), { once: true }) + setTimeout( + () => + resolve({ + ok: true, + status: 200, + headers: new Headers(), + json: () => Promise.resolve(completed), + }), + 10 * 60_000 + ) + }) + ) + + vi.useFakeTimers() + try { + const promise = run(fetchMock) + await vi.advanceTimersByTimeAsync(11 * 60_000) + await expect(promise).resolves.toBeDefined() + } finally { + vi.useRealTimers() + } + }) + + it('does not arm the body deadline on a healthy fast response', async () => { + const completed = { + id: 'resp_1', + status: 'completed', + output: [ + { type: 'message', role: 'assistant', content: [{ type: 'output_text', text: 'ok' }] }, + ], + usage: { input_tokens: 1, output_tokens: 1, total_tokens: 2 }, + } + const fetchMock = vi.fn().mockResolvedValue({ + ok: true, + status: 200, + headers: new Headers(), + json: () => Promise.resolve(completed), + }) + + await expect(run(fetchMock)).resolves.toBeDefined() + expect(fetchMock).toHaveBeenCalledTimes(1) + }) + + /** + * The workflow timeout aborts `request.abortSignal` with `DOMException('timeout', + * 'AbortError')`. It must surface as the caller's abort, never be relabelled as a + * provider body stall — the two have different owners and different fixes. + */ + it('surfaces a workflow timeout as an abort, not as a body stall', async () => { + const workflow = new AbortController() + const fetchMock = vi.fn().mockImplementation((_url: string, init: RequestInit) => { + queueMicrotask(() => workflow.abort(new DOMException('timeout', 'AbortError'))) + return Promise.resolve(stallingBody(init)) + }) + + const error = await run(fetchMock, { abortSignal: workflow.signal }).catch((e) => e) + + expect(fetchMock).toHaveBeenCalledTimes(1) + expect(error.message).not.toContain('response body stalled') + }) +}) diff --git a/apps/sim/providers/openai/core.ts b/apps/sim/providers/openai/core.ts index 47dd9e18b7c..1b78a78ba89 100644 --- a/apps/sim/providers/openai/core.ts +++ b/apps/sim/providers/openai/core.ts @@ -2,6 +2,8 @@ import { createHash } from 'node:crypto' import type { Logger } from '@sim/logger' import { getErrorMessage, toError } from '@sim/utils/errors' import { isRecordLike } from '@sim/utils/object' +import { backoffWithJitter, parseRetryAfter } from '@sim/utils/retry' +import { truncate } from '@sim/utils/string' import type OpenAI from 'openai' import type { NormalizedBlockOutput, StreamingExecution } from '@/executor/types' import { MAX_TOOL_ITERATIONS } from '@/providers' @@ -33,15 +35,164 @@ import { createReadableStreamFromResponses, extractResponseText, extractResponseToolCalls, + isMaxOutputTokensIncompleteResponse, parseResponsesUsage, type ResponsesInputItem, type ResponsesToolCall, + responseContainsFunctionCall, toResponsesToolChoice, } from './utils' +/** + * How long the response body may stall after headers arrive before the attempt is + * abandoned. Generous: the body is tens of KB and follows immediately on a healthy + * call, so this only fires on the stall, well inside the runtime's own ~300s socket + * wall (which is variable, unnamed, and cannot be retried against). + */ +const RESPONSE_BODY_BUDGET_MS = 60_000 + +/** + * Retry budget for a rejected `/v1/responses` request: 2 retries, 3 attempts in + * total. This path posted through the OpenAI SDK until it moved onto raw `fetch`, + * which silently dropped the SDK's own `maxRetries: 2`; every other provider we + * ship still constructs an SDK client and therefore still retries. The number + * matches both the OpenAI SDK and the AI SDK's `_retryWithExponentialBackoff`. + */ +const MAX_RESPONSES_RETRIES = 2 + +/** + * Sub-500 statuses that both the OpenAI SDK and the AI SDK classify as retryable: + * request timeout, lock conflict, and rate limit. + */ +const RETRYABLE_RESPONSE_STATUSES = new Set([408, 409, 429]) + +/** + * A non-2xx reply from `/v1/responses`, carrying the status and any server-supplied + * pacing so the caller can decide on a retry without re-reading a body that has + * already been consumed to build the message. + */ +class ResponsesHttpError extends Error { + readonly status: number + readonly retryAfterMs: number | null + + constructor(message: string, status: number, retryAfterMs: number | null) { + super(message) + this.name = 'ResponsesHttpError' + this.status = status + this.retryAfterMs = retryAfterMs + } +} + +/** + * Whether a rejected request may be re-sent. + * + * Restricted to the classes where the request was refused outright and no response + * was created server-side, so a retry cannot bill a second generation. Everything + * else — including a stalled body, which arrives only after a response exists — is + * surfaced. `/v1/responses` ignores `Idempotency-Key` (verified live: the same key + * with an identical body returns two distinct response ids), so there is no + * deduplication to fall back on and this boundary is the only guard. + */ +function isRetryableResponseStatus(status: number): boolean { + return RETRYABLE_RESPONSE_STATUSES.has(status) || status >= 500 +} + +/** + * Reads server-supplied retry pacing. OpenAI sends `retry-after-ms` alongside the + * standard `Retry-After` on rate limits and it carries sub-second precision, so it + * wins when present and parseable. + */ +function readRetryAfterMs(headers: Headers): number | null { + const raw = headers.get('retry-after-ms') + if (raw !== null) { + const ms = Number(raw.trim()) + if (Number.isFinite(ms) && ms >= 0) return ms + } + return parseRetryAfter(headers.get('retry-after')) +} + +/** + * Waits out a retry backoff, resolving early — and rejecting with the caller's own + * abort reason — the moment the run is cancelled. + * + * A plain `sleep` would hold the provider slot for the full delay after a workflow was + * already cancelled, and the loop would then surface the stale HTTP error rather than + * the cancellation, reporting a cancelled run as a rate limit or a 5xx. + */ +function backoffDelay(ms: number, signal: AbortSignal | undefined): Promise { + return new Promise((resolve, reject) => { + if (signal?.aborted) { + reject(signal.reason) + return + } + const onAbort = () => { + clearTimeout(timer) + reject(signal?.reason) + } + const timer = setTimeout(() => { + signal?.removeEventListener('abort', onAbort) + resolve() + }, ms) + signal?.addEventListener('abort', onAbort, { once: true }) + }) +} + type PreparedTools = ReturnType type ToolChoice = PreparedTools['toolChoice'] +/** + * Rejects a `/v1/responses` body that reports a generation which did not succeed. + * + * The endpoint answers HTTP 200 for failures: `status: 'failed'` with a populated + * `error`, or `status: 'incomplete'` with an `incomplete_details.reason`. Reading + * only `output` therefore reports a failed generation to the user as a success with + * empty content and billed tokens — while `deriveOpenAIFinishReason` independently + * records `finishReason: 'error'` on the same span, so the trace and the block + * contradict each other. + * + * The tolerated case is copied from `streamResponsesTurn` and must keep matching it: + * an `incomplete` response is accepted only when it was truncated by + * `max_output_tokens` AND carries no function call. Truncated prose is still a usable + * partial answer, but a truncated `function_call` holds half-written JSON — executing + * it makes `parseToolArguments` throw, surfacing a confusing tool failure instead of + * the truncation that actually happened. + * + * A status the API did not send is not asserted against: this path is shared with + * Azure OpenAI and any OpenAI-compatible gateway, and inventing a failure for an + * absent field would break healthy responses rather than report broken ones. + */ +function assertUsableResponse(response: OpenAI.Responses.Response, providerLabel: string): void { + if (response.error) { + const code = response.error.code ? ` (${response.error.code})` : '' + throw new Error(`${providerLabel} generation failed${code}: ${response.error.message}`) + } + + if (response.status === 'failed') { + throw new Error( + `${providerLabel} generation failed, and the API returned no error detail explaining why.` + ) + } + + if (response.status === 'incomplete') { + const reason = response.incomplete_details?.reason ?? 'unknown' + if (responseContainsFunctionCall(response)) { + throw new Error( + `${providerLabel} generation stopped before completion (${reason}), truncating a tool call mid-argument. Raise the max output tokens or reduce the tool schema size.` + ) + } + if (!isMaxOutputTokensIncompleteResponse(response)) { + throw new Error(`${providerLabel} generation stopped before completion: ${reason}.`) + } + return + } + + if (response.status && response.status !== 'completed') { + throw new Error( + `${providerLabel} returned a response with status "${response.status}", which carries no finished generation.` + ) + } +} + /** * Stable routing key for OpenAI's prompt cache, scoped to one agent block. * @@ -85,6 +236,12 @@ export async function executeResponsesProviderRequest( logger.info(`Preparing ${config.providerLabel} request`, { model: request.model, + // Correlation ids: without these a provider call cannot be tied back to the + // execution that issued it, which leaves a stalled request indistinguishable + // from one that was never made. + workflowId: request.workflowId, + blockId: request.blockId, + executionId: request.executionId, hasSystemPrompt: !!request.systemPrompt, hasMessages: !!request.messages?.length, hasTools: !!request.tools?.length, @@ -237,14 +394,18 @@ export async function executeResponsesProviderRequest( ...overrides, }) + /** + * A non-JSON body here is usually a gateway/CDN HTML page, and this string reaches the + * user-facing block error and the trace span — so it is bounded rather than pasted in + * whole. Falls back to `statusText` when the body carries nothing useful. + */ const parseErrorResponse = async (response: Response): Promise => { - const text = await response.text() + const text = await response.text().catch(() => '') try { const payload = JSON.parse(text) - return payload?.error?.message || text - } catch { - return text - } + if (payload?.error?.message) return payload.error.message + } catch {} + return truncate(text.trim(), 500) || response.statusText || `HTTP ${response.status}` } /** @@ -270,27 +431,66 @@ export async function executeResponsesProviderRequest( let reasoningSummariesUnavailable = false - const fetchResponsesWithSummaryFallback = async ( + /** + * One POST, paired with a deadline that can bound any body read on the response. + * + * The deadline is created here rather than by the caller because a non-2xx body is + * read inside this function, before the caller ever sees the response — an error body + * that stalls would otherwise hang unbounded until the runtime's socket wall, which is + * exactly the failure this change exists to remove. + */ + const postOnce = async ( + bodyToSend: Record, + abortSignal: AbortSignal | undefined + ): Promise<{ response: Response; bodyDeadline: AbortController }> => { + const bodyDeadline = new AbortController() + const signal = abortSignal + ? AbortSignal.any([abortSignal, bodyDeadline.signal]) + : bodyDeadline.signal + const response = await fetchImpl(config.endpoint, { + method: 'POST', + headers: config.headers, + body: JSON.stringify(bodyToSend), + signal, + }) + return { response, bodyDeadline } + } + + /** Reads a non-2xx body under the same deadline that bounds a successful one. */ + const readErrorBody = async ( + response: Response, + bodyDeadline: AbortController + ): Promise => { + const timer = setTimeout(() => { + bodyDeadline.abort(new DOMException('response body stalled', 'TimeoutError')) + }, RESPONSE_BODY_BUDGET_MS) + try { + return await parseErrorResponse(response) + } finally { + clearTimeout(timer) + } + } + + const fetchResponsesAttempt = async ( requestedBody: Record, - abortSignal = request.abortSignal + abortSignal: AbortSignal | undefined ): Promise => { const body = reasoningSummariesUnavailable ? (stripReasoningSummary(requestedBody) ?? requestedBody) : requestedBody - const response = await fetchImpl(config.endpoint, { - method: 'POST', - headers: config.headers, - body: JSON.stringify(body), - signal: abortSignal, - }) + const { response, bodyDeadline } = await postOnce(body, abortSignal) if (response.ok) return response - const message = await parseErrorResponse(response) + const message = await readErrorBody(response, bodyDeadline) const strippedBody = isReasoningSummaryVerificationError(response.status, message) ? stripReasoningSummary(body) : null if (!strippedBody) { - throw new Error(`${config.providerLabel} API error (${response.status}): ${message}`) + throw new ResponsesHttpError( + `${config.providerLabel} API error (${response.status}): ${message}`, + response.status, + readRetryAfterMs(response.headers) + ) } reasoningSummariesUnavailable = true @@ -298,26 +498,200 @@ export async function executeResponsesProviderRequest( `${config.providerLabel} rejected reasoning summaries (organization not verified); retrying without summary`, { model: config.modelName } ) - const retryResponse = await fetchImpl(config.endpoint, { - method: 'POST', - headers: config.headers, - body: JSON.stringify(strippedBody), - signal: abortSignal, - }) + const { response: retryResponse, bodyDeadline: retryDeadline } = await postOnce( + strippedBody, + abortSignal + ) if (!retryResponse.ok) { - const retryMessage = await parseErrorResponse(retryResponse) - throw new Error( - `${config.providerLabel} API error (${retryResponse.status}): ${retryMessage}` + const retryMessage = await readErrorBody(retryResponse, retryDeadline) + throw new ResponsesHttpError( + `${config.providerLabel} API error (${retryResponse.status}): ${retryMessage}`, + retryResponse.status, + readRetryAfterMs(retryResponse.headers) ) } return retryResponse } + /** + * Sends one Responses request, re-sending it on a refusal that created nothing + * server-side (408/409/429/5xx) with exponential backoff and any `Retry-After` + * the server supplied. + * + * The retry lives here rather than in `postResponses` so the streaming paths are + * covered too: a rejected request never yields a body, so no stream bytes have + * been handed to a consumer and no generation has started. Only transport + * failures reach the caller unretried — an abort belongs to whoever raised it, + * and a stalled body arrives after a response already exists, which makes it the + * one class a retry would double-bill. + */ + const fetchResponsesWithSummaryFallback = async ( + requestedBody: Record, + abortSignal = request.abortSignal + ): Promise => { + for (let attempt = 1; ; attempt++) { + try { + return await fetchResponsesAttempt(requestedBody, abortSignal) + } catch (error) { + /** + * A cancelled run reports the cancellation, never the status that happened to be + * in flight when it was cancelled — surfacing the stale error would report a + * cancelled run as a rate limit or a 5xx. + */ + if (abortSignal?.aborted) { + throw abortSignal.reason ?? error + } + + const exhausted = attempt > MAX_RESPONSES_RETRIES + if ( + exhausted || + !(error instanceof ResponsesHttpError) || + !isRetryableResponseStatus(error.status) + ) { + throw error + } + + const delayMs = backoffWithJitter(attempt, error.retryAfterMs) + logger.warn(`${config.providerLabel} request failed with a retryable status; retrying`, { + attempt, + status: error.status, + delayMs, + model: config.modelName, + workflowId: request.workflowId, + blockId: request.blockId, + executionId: request.executionId, + }) + await backoffDelay(delayMs, abortSignal) + } + } + } + + /** + * Annotates an opaque transport failure with the request phase it died in. + * + * A model call that stalls surfaces only the runtime's own message — Bun's + * `TimeoutError: The operation timed out.` — which is indistinguishable between + * "the request was never answered" and "the response arrived but its body never + * completed". Those have opposite causes and opposite fixes, and the difference + * is only observable from inside the call. + * + * The phase is folded into the error message rather than logged alone because + * the message reaches the block's trace span, and the trace persists even when a + * task has stopped shipping logs. Errors that already describe themselves (an + * API error carrying a status and provider message) are left untouched; only + * `TimeoutError`/`AbortError`, which name nothing, are annotated. + */ + const annotateTransportFailure = ( + error: unknown, + phase: 'awaiting-response-headers' | 'reading-response-body', + startedAt: number, + detail?: Record + ): unknown => { + if (!(error instanceof Error)) return error + if (error.name !== 'TimeoutError' && error.name !== 'AbortError') return error + + const elapsedMs = Date.now() - startedAt + const fields = Object.entries(detail ?? {}) + .filter(([, value]) => value !== null && value !== undefined) + .map(([key, value]) => `${key}=${value}`) + const context = [`phase=${phase}`, `elapsedMs=${elapsedMs}`, ...fields].join(' ') + + logger.error(`${config.providerLabel} request failed in transport`, { + phase, + elapsedMs, + errorName: error.name, + model: config.modelName, + workflowId: request.workflowId, + blockId: request.blockId, + executionId: request.executionId, + ...detail, + }) + + /** + * A new Error rather than a mutation: the runtime raises these as `DOMException`, + * whose `message` is a readonly getter, so assigning to it throws a `TypeError` and + * destroys the very failure being reported. + * + * `name` is copied and the original hangs off `cause` so the classification survives + * — this error is rewrapped in a `ProviderError` further up, which overwrites `name`, + * and the agent handler reads the cause to recognise a transport timeout. + */ + const annotated = new Error(`${error.message} [${context}]`, { cause: error }) + annotated.name = error.name + return annotated + } + const postResponses = async ( body: Record ): Promise => { - const response = await fetchResponsesWithSummaryFallback(body) - return response.json() + const startedAt = Date.now() + + /** + * Bounds the body read only — never time-to-headers. + * + * `/v1/responses` withholds its 200 until generation is finished, so the whole + * think time lands in the headers phase and the body then transfers in about a + * millisecond (measured: a 14.5s call spent 14545ms to headers and 1ms on the body). + * Leaving headers unbounded therefore costs nothing and keeps slow reasoning models + * working, while a stalled body — the documented failure where the 200 arrives and + * the bytes never follow — is caught here instead of by the runtime's own ~300s + * socket wall, which is variable, unnamed, and fires far too late to be useful. + * + * The failure is surfaced, not retried: `/v1/responses` ignores `Idempotency-Key` + * (verified against the live API — the same key with an identical body returns two + * distinct response ids and a conflicting body draws no 409), so a retry would + * generate and bill a second response. Both the OpenAI SDK and the AI SDK likewise + * decline to retry this class; the AI SDK classifies `TimeoutError` as an abort and + * rethrows it. + * + * Scope: non-streaming only. The streaming path holds its body open by design, so a + * deadline there would cut healthy generations; it keeps the runtime's socket wall. + */ + const bodyDeadline = new AbortController() + const signal = request.abortSignal + ? AbortSignal.any([request.abortSignal, bodyDeadline.signal]) + : bodyDeadline.signal + + let response: Response + try { + response = await fetchResponsesWithSummaryFallback(body, signal) + } catch (error) { + throw annotateTransportFailure(error, 'awaiting-response-headers', startedAt) + } + + /** + * `x-request-id` is the only handle OpenAI support can trace a call by, so it is + * captured here — a stalled request is exactly the case where we need to hand them + * one, and it is unavailable once the body read fails. + */ + const responseMeta = { + status: response.status, + ttfbMs: Date.now() - startedAt, + requestId: response.headers.get('x-request-id'), + contentLength: response.headers.get('content-length'), + contentEncoding: response.headers.get('content-encoding'), + } + + const timer = setTimeout(() => { + bodyDeadline.abort(new DOMException('response body stalled', 'TimeoutError')) + }, RESPONSE_BODY_BUDGET_MS) + + let parsed: OpenAI.Responses.Response + try { + parsed = await response.json() + } catch (error) { + throw annotateTransportFailure(error, 'reading-response-body', startedAt, responseMeta) + } finally { + clearTimeout(timer) + } + + /** + * Asserted here rather than at the call sites so every non-streaming turn — the + * first and each tool-loop continuation — is covered by construction, and outside + * the transport `try` so a rejected generation is never mistaken for a body stall. + */ + assertUsableResponse(parsed, config.providerLabel) + return parsed } const providerStartTime = Date.now() @@ -476,16 +850,38 @@ export async function executeResponsesProviderRequest( content = responseText } - const toolCallsInResponse = extractResponseToolCalls(currentResponse.output) + const emittedToolCalls = extractResponseToolCalls(currentResponse.output) enrichLastModelSegmentFromOpenAIResponse( timeSegments, currentResponse, responseText, - toolCallsInResponse, + emittedToolCalls, { model: request.model } ) + /** + * Mirrors `toolsExecutable` in the streaming tool loop: a tool call only runs + * when it came from a finished generation. + * + * Unreachable today, and deliberately kept. `assertUsableResponse` already + * rejects every status that could carry a tool call from an unfinished + * generation — and because both it and `extractResponseToolCalls` key off the + * same `function_call` output item, no response can reach here non-completed + * with a tool call to run. It stays as the second lock on the invariant: these + * two loops diverging on exactly this check is what produced the bug, and a + * later relaxation of the assert would otherwise re-open it silently. + */ + const toolsExecutable = !currentResponse.status || currentResponse.status === 'completed' + const toolCallsInResponse = toolsExecutable ? emittedToolCalls : [] + + if (emittedToolCalls.length > 0 && !toolsExecutable) { + logger.warn('Skipping OpenAI tool execution', { + status: currentResponse.status, + toolCount: emittedToolCalls.length, + }) + } + if (!toolCallsInResponse.length) { break } @@ -722,10 +1118,14 @@ export async function executeResponsesProviderRequest( throw error } - throw new ProviderError(toError(error).message, { - startTime: providerStartTimeISO, - endTime: providerEndTimeISO, - duration: totalDuration, - }) + throw new ProviderError( + toError(error).message, + { + startTime: providerStartTimeISO, + endTime: providerEndTimeISO, + duration: totalDuration, + }, + { cause: error } + ) } } diff --git a/apps/sim/providers/types.ts b/apps/sim/providers/types.ts index 7c402d66677..e029f830d2c 100644 --- a/apps/sim/providers/types.ts +++ b/apps/sim/providers/types.ts @@ -241,8 +241,17 @@ export class ProviderError extends Error { duration: number } - constructor(message: string, timing: { startTime: string; endTime: string; duration: number }) { - super(message) + /** + * `options.cause` should carry the error being wrapped. `name` is deliberately + * overwritten with `'ProviderError'`, so without a cause every classification the + * original carried — notably a transport `TimeoutError` — is lost to callers. + */ + constructor( + message: string, + timing: { startTime: string; endTime: string; duration: number }, + options?: ErrorOptions + ) { + super(message, options) this.name = 'ProviderError' this.timing = timing }