From 907f1fa36fe834f93117a9ce041e0300c5d8537c Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Tue, 11 Aug 2026 11:10:25 -0700 Subject: [PATCH 1/3] fix(tools): bound internal tool calls by the plan deadline, not Bun's fetch default --- .../sim/lib/core/utils/fetch-deadline.test.ts | 63 +++++++++++++++++++ apps/sim/lib/core/utils/fetch-deadline.ts | 63 +++++++++++++++++++ apps/sim/tools/index.ts | 36 +++++++++-- 3 files changed, 156 insertions(+), 6 deletions(-) create mode 100644 apps/sim/lib/core/utils/fetch-deadline.test.ts create mode 100644 apps/sim/lib/core/utils/fetch-deadline.ts 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..7b335f8de07 --- /dev/null +++ b/apps/sim/lib/core/utils/fetch-deadline.test.ts @@ -0,0 +1,63 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { isTransportTimeoutError, withFetchDeadline } from '@/lib/core/utils/fetch-deadline' + +describe('withFetchDeadline', () => { + it('states the caller deadline as the transport deadline', () => { + expect(withFetchDeadline({ method: 'POST' }, 3_000_000).timeout).toBe(3_000_000) + }) + + it('preserves the init the caller already built', () => { + const signal = new AbortController().signal + const init = withFetchDeadline({ method: 'POST', body: 'x', signal }, 1000) + expect(init.method).toBe('POST') + expect(init.body).toBe('x') + expect(init.signal).toBe(signal) + }) + + it('rounds a fractional deadline up rather than down', () => { + expect(withFetchDeadline({}, 1500.2).timeout).toBe(1501) + }) + + /* + * The bug this module exists for: an absent application deadline must disable + * the transport timer, never fall back to the runtime's 300s default. + */ + it.each([ + ['undefined', undefined], + ['zero', 0], + ['negative', -1], + ['Infinity', Number.POSITIVE_INFINITY], + ['NaN', Number.NaN], + ])('disables the transport timer when the deadline is %s', (_label, deadline) => { + expect(withFetchDeadline({}, deadline as number | undefined).timeout).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..1bc930db3fb --- /dev/null +++ b/apps/sim/lib/core/utils/fetch-deadline.ts @@ -0,0 +1,63 @@ +/** + * Keeps the transport deadline from undercutting the application deadline. + * + * Bun's HTTP client arms an idle timer defaulting to 300s. It re-arms on writes + * and body-phase reads, but *not* on response-header reads — so it is an + * absolute deadline for the peer to begin answering. An `AbortSignal` cannot + * raise it, which makes it invisible to every caller that believes it owns the + * deadline: a request whose peer legitimately works before it replies dies at + * five minutes no matter what timeout was 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. + * + * 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` disables the timer entirely; a positive number is the idle + * deadline in milliseconds. + */ +export interface DeadlineRequestInit extends RequestInit { + timeout?: number | boolean +} + +/** + * Applies `deadlineMs` as the transport idle deadline alongside whatever + * `AbortSignal` the caller already set, so both layers express one number. + * + * Pass the same deadline the caller enforces in-process. A non-finite or + * non-positive deadline means "no application bound", which disables the + * transport timer rather than silently falling back to Bun's 300s default — + * falling back is what produced the bug this exists to prevent. + */ +export function withFetchDeadline( + init: RequestInit, + deadlineMs: number | undefined +): DeadlineRequestInit { + if (deadlineMs === undefined || !Number.isFinite(deadlineMs) || deadlineMs <= 0) { + return { ...init, timeout: false } + } + return { ...init, timeout: Math.ceil(deadlineMs) } +} + +/** + * 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..618bb2ec371 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, withFetchDeadline } 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,20 @@ async function executeToolRequest( } } + const attemptStartedAt = Date.now() try { - const internalResponse = await fetch(fullUrl, { - method: requestParams.method, - headers: headers, - body: requestParams.body, - signal: controller.signal, - }) + const internalResponse = await fetch( + fullUrl, + withFetchDeadline( + { + method: requestParams.method, + headers: headers, + body: requestParams.body, + signal: controller.signal, + }, + timeout + ) + ) if ( nullBodyStatuses.has(internalResponse.status) || shouldRetryWithoutReadingBody( @@ -2493,6 +2501,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) From 3d1080361c5bff5bd857d07cc10936413009b9f3 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Tue, 11 Aug 2026 11:24:56 -0700 Subject: [PATCH 2/3] fix(tools): disarm Bun's fetch idle timer instead of passing a numeric deadline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bun 1.3.14 ignores a positive numeric `timeout` on fetch and honors only the boolean/zero form, so passing the plan deadline through changed nothing and internal tool calls still died at the 300s default. Verified against the pinned runtime: `{ timeout: 1000 }` does not abort a request that takes 3s to answer, and `BUN_CONFIG_HTTP_IDLE_TIMEOUT` has no effect either — both are `main`-only. The caller on this path already arms an AbortController with the plan timeout, so the transport timer is disarmed rather than re-negotiated, leaving one enforcement point instead of two that disagree. Co-Authored-By: Claude Opus 5 (1M context) --- .../sim/lib/core/utils/fetch-deadline.test.ts | 35 ++++++------- apps/sim/lib/core/utils/fetch-deadline.ts | 49 ++++++++++--------- apps/sim/tools/index.ts | 22 +++++---- 3 files changed, 53 insertions(+), 53 deletions(-) diff --git a/apps/sim/lib/core/utils/fetch-deadline.test.ts b/apps/sim/lib/core/utils/fetch-deadline.test.ts index 7b335f8de07..750616ce1f7 100644 --- a/apps/sim/lib/core/utils/fetch-deadline.test.ts +++ b/apps/sim/lib/core/utils/fetch-deadline.test.ts @@ -2,37 +2,30 @@ * @vitest-environment node */ import { describe, expect, it } from 'vitest' -import { isTransportTimeoutError, withFetchDeadline } from '@/lib/core/utils/fetch-deadline' +import { isTransportTimeoutError, withCallerOwnedDeadline } from '@/lib/core/utils/fetch-deadline' -describe('withFetchDeadline', () => { - it('states the caller deadline as the transport deadline', () => { - expect(withFetchDeadline({ method: 'POST' }, 3_000_000).timeout).toBe(3_000_000) +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 = withFetchDeadline({ method: 'POST', body: 'x', signal }, 1000) + const init = withCallerOwnedDeadline({ method: 'POST', body: 'x', signal }) expect(init.method).toBe('POST') expect(init.body).toBe('x') expect(init.signal).toBe(signal) }) - it('rounds a fractional deadline up rather than down', () => { - expect(withFetchDeadline({}, 1500.2).timeout).toBe(1501) - }) - - /* - * The bug this module exists for: an absent application deadline must disable - * the transport timer, never fall back to the runtime's 300s default. - */ - it.each([ - ['undefined', undefined], - ['zero', 0], - ['negative', -1], - ['Infinity', Number.POSITIVE_INFINITY], - ['NaN', Number.NaN], - ])('disables the transport timer when the deadline is %s', (_label, deadline) => { - expect(withFetchDeadline({}, deadline as number | undefined).timeout).toBe(false) + it('does not mutate the caller’s init', () => { + const original: RequestInit = { method: 'POST' } + withCallerOwnedDeadline(original) + expect('timeout' in original).toBe(false) }) }) diff --git a/apps/sim/lib/core/utils/fetch-deadline.ts b/apps/sim/lib/core/utils/fetch-deadline.ts index 1bc930db3fb..0e8cb76fab1 100644 --- a/apps/sim/lib/core/utils/fetch-deadline.ts +++ b/apps/sim/lib/core/utils/fetch-deadline.ts @@ -1,12 +1,11 @@ /** * Keeps the transport deadline from undercutting the application deadline. * - * Bun's HTTP client arms an idle timer defaulting to 300s. It re-arms on writes - * and body-phase reads, but *not* on response-header reads — so it is an - * absolute deadline for the peer to begin answering. An `AbortSignal` cannot - * raise it, which makes it invisible to every caller that believes it owns the - * deadline: a request whose peer legitimately works before it replies dies at - * five minutes no matter what timeout was computed for it. + * 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 @@ -14,36 +13,42 @@ * 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. + * + * Note the pinned runtime accepts only the boolean/zero form. Bun 1.3.14 + * ignores a positive numeric `timeout` — verified against the pinned version by + * observing that `{ timeout: 1000 }` does not abort a request that takes 3s to + * answer — so passing the deadline as a number silently changes nothing. The + * numeric idle-deadline form exists only on Bun's `main`. Do not "improve" this + * to pass the deadline through until the pinned version supports it, and + * re-verify with that probe if you do. + * * 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` disables the timer entirely; a positive number is the idle - * deadline in milliseconds. + * declare. `false` disarms the timer; `true` or omitted keeps the default. */ export interface DeadlineRequestInit extends RequestInit { timeout?: number | boolean } /** - * Applies `deadlineMs` as the transport idle deadline alongside whatever - * `AbortSignal` the caller already set, so both layers express one number. + * Disarms the transport idle timer so the caller's own deadline is the only one + * in force. * - * Pass the same deadline the caller enforces in-process. A non-finite or - * non-positive deadline means "no application bound", which disables the - * transport timer rather than silently falling back to Bun's 300s default — - * falling back is what produced the bug this exists to prevent. + * 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 withFetchDeadline( - init: RequestInit, - deadlineMs: number | undefined -): DeadlineRequestInit { - if (deadlineMs === undefined || !Number.isFinite(deadlineMs) || deadlineMs <= 0) { - return { ...init, timeout: false } - } - return { ...init, timeout: Math.ceil(deadlineMs) } +export function withCallerOwnedDeadline(init: RequestInit): DeadlineRequestInit { + return { ...init, timeout: false } } /** diff --git a/apps/sim/tools/index.ts b/apps/sim/tools/index.ts index 618bb2ec371..04e2b11525c 100644 --- a/apps/sim/tools/index.ts +++ b/apps/sim/tools/index.ts @@ -24,7 +24,7 @@ import { validateUrlWithDNS, } from '@/lib/core/security/input-validation.server' import { PlatformEvents } from '@/lib/core/telemetry' -import { isTransportTimeoutError, withFetchDeadline } from '@/lib/core/utils/fetch-deadline' +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 { @@ -2444,17 +2444,19 @@ async function executeToolRequest( const attemptStartedAt = Date.now() try { + /* + * `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, - withFetchDeadline( - { - method: requestParams.method, - headers: headers, - body: requestParams.body, - signal: controller.signal, - }, - timeout - ) + withCallerOwnedDeadline({ + method: requestParams.method, + headers: headers, + body: requestParams.body, + signal: controller.signal, + }) ) if ( nullBodyStatuses.has(internalResponse.status) || From 42bd20e67fb1cc7fe7798bdcbbc29f11dc17facb Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Tue, 11 Aug 2026 11:58:49 -0700 Subject: [PATCH 3/3] docs(tools): record the measured Bun 1.3.14 timeout behavior Replaces the inferred note with the numbers from a probe against the pinned runtime: no option dies at 300028ms, timeout:false survives 310031ms, and a numeric timeout is ignored. Also records that bun-types@1.3.14 does not declare the option even though the runtime honors it, which is why the interface is declared locally. Co-Authored-By: Claude Opus 5 (1M context) --- apps/sim/lib/core/utils/fetch-deadline.ts | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/apps/sim/lib/core/utils/fetch-deadline.ts b/apps/sim/lib/core/utils/fetch-deadline.ts index 0e8cb76fab1..9673fdfac11 100644 --- a/apps/sim/lib/core/utils/fetch-deadline.ts +++ b/apps/sim/lib/core/utils/fetch-deadline.ts @@ -19,13 +19,22 @@ * exactly the bug. Disarming leaves one enforcement point instead of two that * disagree. * - * Note the pinned runtime accepts only the boolean/zero form. Bun 1.3.14 - * ignores a positive numeric `timeout` — verified against the pinned version by - * observing that `{ timeout: 1000 }` does not abort a request that takes 3s to - * answer — so passing the deadline as a number silently changes nothing. The - * numeric idle-deadline form exists only on Bun's `main`. Do not "improve" this - * to pass the deadline through until the pinned version supports it, and - * re-verify with that probe if you do. + * 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.