diff --git a/apps/sim/lib/core/utils/fetch-deadline.test.ts b/apps/sim/lib/core/utils/fetch-deadline.test.ts new file mode 100644 index 00000000000..750616ce1f7 --- /dev/null +++ b/apps/sim/lib/core/utils/fetch-deadline.test.ts @@ -0,0 +1,56 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { isTransportTimeoutError, withCallerOwnedDeadline } from '@/lib/core/utils/fetch-deadline' + +describe('withCallerOwnedDeadline', () => { + /* + * The pinned Bun ignores a positive numeric `timeout` and honors only the + * boolean/zero form, so anything other than `false` here silently leaves the + * 300s default in force — which is the outage this module exists to prevent. + */ + it('disarms the transport timer rather than negotiating a value', () => { + expect(withCallerOwnedDeadline({}).timeout).toBe(false) + }) + + it('preserves the init the caller already built', () => { + const signal = new AbortController().signal + const init = withCallerOwnedDeadline({ method: 'POST', body: 'x', signal }) + expect(init.method).toBe('POST') + expect(init.body).toBe('x') + expect(init.signal).toBe(signal) + }) + + it('does not mutate the caller’s init', () => { + const original: RequestInit = { method: 'POST' } + withCallerOwnedDeadline(original) + expect('timeout' in original).toBe(false) + }) +}) + +describe('isTransportTimeoutError', () => { + it('recognizes the runtime timeout', () => { + const error = new Error('The operation timed out.') + error.name = 'TimeoutError' + expect(isTransportTimeoutError(error)).toBe(true) + }) + + it('recognizes a severed connection', () => { + expect(isTransportTimeoutError(new TypeError('fetch failed'))).toBe(true) + }) + + it('does not claim a cancellation', () => { + const error = new Error('aborted') + error.name = 'AbortError' + expect(isTransportTimeoutError(error)).toBe(false) + }) + + it.each([ + ['an unrelated TypeError', new TypeError('x is not a function')], + ['a plain error', new Error('boom')], + ['a non-error', 'fetch failed'], + ])('does not claim %s', (_label, value) => { + expect(isTransportTimeoutError(value)).toBe(false) + }) +}) diff --git a/apps/sim/lib/core/utils/fetch-deadline.ts b/apps/sim/lib/core/utils/fetch-deadline.ts new file mode 100644 index 00000000000..9673fdfac11 --- /dev/null +++ b/apps/sim/lib/core/utils/fetch-deadline.ts @@ -0,0 +1,77 @@ +/** + * Keeps the transport deadline from undercutting the application deadline. + * + * Bun's HTTP client arms an idle timer defaulting to 300s. It is not raised by + * an `AbortSignal`, and it does not re-arm while awaiting response headers, so + * it acts as an absolute deadline for the peer to begin answering. Any request + * whose peer legitimately works before it replies dies at five minutes no + * matter what deadline the caller computed for it. + * + * This bit production. Workflow function blocks are bounded by a plan deadline + * (50 minutes on enterprise), but the executor's call into the internal + * function route inherited Bun's default instead, so every sandbox run longer + * than five minutes failed with a bare `fetch failed` that read as user-code + * failure rather than a transport cap. + * + * The timer is therefore disarmed rather than re-negotiated: callers on this + * path already own an in-process deadline (an `AbortController` armed with the + * plan timeout), and a second, shorter, invisible deadline underneath it is + * exactly the bug. Disarming leaves one enforcement point instead of two that + * disagree. + * + * The pinned runtime accepts only the boolean/zero form. Measured on Bun 1.3.14 + * against a server that withholds response headers, so the numbers below are + * the real deadline rather than an inferred one: + * + * no option -> THREW 300028ms (TimeoutError) <- the 300s default + * timeout: false -> RESOLVED 310031ms <- disarmed + * timeout: 1000 -> RESOLVED 3008ms on a 3s request <- numeric ignored + * + * So a positive numeric `timeout` silently changes nothing on this version; the + * numeric idle-deadline form and `BUN_CONFIG_HTTP_IDLE_TIMEOUT` both exist only + * on Bun's `main`. Do not "improve" this into a numeric pass-through until the + * pinned version supports it, and re-measure with the probe above if you do. + * + * `bun-types@1.3.14` does not declare `timeout` on `BunFetchRequestInit` even + * though the runtime honors the boolean form — the types lag the runtime, which + * is why the interface below is declared locally rather than imported. + * + * Node's undici has no equivalent default and ignores the option, so this is + * safe on both runtimes. + */ + +/** + * `RequestInit` plus Bun's idle-timeout control, which the DOM lib does not + * declare. `false` disarms the timer; `true` or omitted keeps the default. + */ +export interface DeadlineRequestInit extends RequestInit { + timeout?: number | boolean +} + +/** + * Disarms the transport idle timer so the caller's own deadline is the only one + * in force. + * + * Only use this where the caller genuinely enforces a deadline in-process — + * an `AbortSignal` wired to a timer or an execution budget. Without one, a + * request to a peer that never answers would hang until the socket dies. + */ +export function withCallerOwnedDeadline(init: RequestInit): DeadlineRequestInit { + return { ...init, timeout: false } +} + +/** + * Whether a caught error is the transport giving up rather than the request + * being cancelled or the peer erroring. + * + * Bun reports both an unanswered request and a truncated body as + * `TimeoutError: The operation timed out.`, and surfaces a severed connection + * as a bare `fetch failed` — none of which name the hop, the elapsed time, or + * the fact that a cap was hit. Callers use this to annotate before rethrowing + * so a transport cap cannot masquerade as a failure of the work itself. + */ +export function isTransportTimeoutError(error: unknown): error is Error { + if (!(error instanceof Error)) return false + if (error.name === 'TimeoutError') return true + return error.name === 'TypeError' && error.message === 'fetch failed' +} diff --git a/apps/sim/tools/index.ts b/apps/sim/tools/index.ts index 26e63537fb4..04e2b11525c 100644 --- a/apps/sim/tools/index.ts +++ b/apps/sim/tools/index.ts @@ -24,6 +24,7 @@ import { validateUrlWithDNS, } from '@/lib/core/security/input-validation.server' import { PlatformEvents } from '@/lib/core/telemetry' +import { isTransportTimeoutError, withCallerOwnedDeadline } from '@/lib/core/utils/fetch-deadline' import { HttpError } from '@/lib/core/utils/http-error' import { generateRequestId } from '@/lib/core/utils/request' import { @@ -2441,13 +2442,22 @@ async function executeToolRequest( } } + const attemptStartedAt = Date.now() try { - const internalResponse = await fetch(fullUrl, { - method: requestParams.method, - headers: headers, - body: requestParams.body, - signal: controller.signal, - }) + /* + * `controller` above is armed with `timeout`, so the plan deadline is + * already enforced in-process; the transport timer is disarmed so its + * 300s default cannot undercut it. + */ + const internalResponse = await fetch( + fullUrl, + withCallerOwnedDeadline({ + method: requestParams.method, + headers: headers, + body: requestParams.body, + signal: controller.signal, + }) + ) if ( nullBodyStatuses.has(internalResponse.status) || shouldRetryWithoutReadingBody( @@ -2493,6 +2503,22 @@ async function executeToolRequest( } throw new Error(`Request timed out after ${timeout}ms`) } + /* + * A transport give-up names neither the hop nor the elapsed time, so + * it reads as a failure of the work the route was doing rather than + * of the call to it. Say which it was before rethrowing. + * + * Keep the original message in the text: `isRetryableFailure` above + * classifies by substring, so dropping it would silently reclassify + * a retryable `timed out` as non-retryable. + */ + if (isTransportTimeoutError(error)) { + throw new Error( + `Transport failure calling ${toolId} after ${Date.now() - attemptStartedAt}ms ` + + `(deadline ${timeout}ms): ${error.message}`, + { cause: error } + ) + } throw error } finally { clearTimeout(timeoutId)