Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions apps/sim/lib/core/utils/fetch-deadline.test.ts
Original file line numberDiff line numberDiff line change
@@ -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)
})
})
77 changes: 77 additions & 0 deletions apps/sim/lib/core/utils/fetch-deadline.ts
Original file line numberDiff line numberDiff line change
@@ -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'
}
38 changes: 32 additions & 6 deletions apps/sim/tools/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 {
Expand DownExpand Up@@ -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(
Expand DownExpand Up@@ -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)
Expand Down
Loading