Skip to content

fix: default timeout on outbound fetch calls - #392

Open
hugo-ccabral wants to merge 2 commits into
mainfrom
fix/fetch-default-timeout
Open

fix: default timeout on outbound fetch calls#392
hugo-ccabral wants to merge 2 commits into
mainfrom
fix/fetch-default-timeout

Conversation

@hugo-ccabral

@hugo-ccabral hugo-ccabral commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Nothing bounded how long an outbound fetch() could hang. withInflightTimeout only freed the dedup-cache Map slot on a stuck promise — the underlying fetch kept running abandoned, pinning isolate memory until exceededMemory killed the worker (514 crashes/24h observed on one storefront).
  • Adds packages/blocks/src/sdk/fetchTimeout.ts: withFetchTimeout() / withTimeoutSignal(), using native AbortSignal.timeout() composed with AbortSignal.any() so a caller-supplied signal is never dropped. Default 10s (DEFAULT_FETCH_TIMEOUT_MS).
  • Wires it into createInstrumentedFetch (the shared VTEX/Shopify chokepoint) as timeoutMs, overridable per-integration at creation and per-call via init.timeoutMs.
  • Applies it directly at every raw fetch() call site that bypassed instrumentation: VTEX _fetch fallback + fetchSafe/fetchAPI, Magento's magentoFetch, Resend's sendEmail, Shopify's createGraphqlClient fallback, and the Google Fonts loader.

Out of scope (flagged, not fixed here)

  • abTesting.ts's proxyToFallback streams the full incoming request body (duplex: "half") to a fallback origin — a blanket default risks breaking legitimate large uploads and needs its own design pass.
  • Browser-only hooks (useCart, useUser, etc.) and invoke.ts — client-side fetches to the site's own origin, not the server-side memory-pinning failure mode this fixes.

Test plan

  • bun run --filter='./packages/blocks' typecheck and all touched packages typecheck clean
  • bun run test on blocks, apps-vtex, apps-magento, apps-resend, apps-website, apps-shopify — all pass (blocks' 23 pre-existing URLPattern failures confirmed unrelated via git stash)
  • Updated apps-resend/send.test.ts's exact toHaveBeenCalledWith assertion to allow the new signal field

🤖 Generated with Claude Code


Summary by cubic

Add a default 10s timeout to all outbound fetch calls to stop hung requests from pinning memory and crashing workers. The timeout uses AbortSignal.timeout() and composes with any caller signal.

  • Bug Fixes

    • Added @decocms/blocks helpers: withFetchTimeout(), withTimeoutSignal(), DEFAULT_FETCH_TIMEOUT_MS = 10_000, and a FetchFn alias.
    • Integrated into createInstrumentedFetch with timeoutMs (per-integration) and init.timeoutMs (per-call).
    • Applied to direct fetches in apps-vtex (incl. sitemap), apps-magento, apps-resend, apps-shopify, and apps-website (Google Fonts). Added Biome rule to ban bare fetch in commerce packages. Updated Resend test to accept a signal.
  • Migration

    • No action needed. For long-lived/streaming requests, disable or adjust via timeoutMs (set to 0/Infinity or a higher value).

Written for commit 70833a6. Summary will update on new commits.

Review in cubic

Nothing bounded how long an outbound fetch could hang. withInflightTimeout
only freed the dedup-cache Map slot on a stuck promise -- the real fetch kept
running abandoned, pinning isolate memory until exceededMemory killed the
worker (514 crashes/24h on one storefront). This aborts the request itself
via AbortSignal.timeout, composed with any caller-supplied signal, wired into
createInstrumentedFetch (per-integration + per-call override) and every raw
fetch call site that bypassed it (VTEX, Magento, Resend, Shopify, fonts).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…apper

A future dev can still bypass fetchSafe/vtexFetch/magentoFetch/etc. by typing
bare fetch(), silently losing the timeout. Adds a Biome noRestrictedGlobals
override banning the fetch global in apps-vtex/apps-magento/apps-resend/
apps-shopify/apps-website (excluding client-side hooks and tests), pointing
at the sanctioned client.

The ban also catches `typeof fetch` in type positions, so introduces a
FetchFn alias (blocks/src/sdk/fetchTimeout.ts, exported from sdk/index.ts) to
replace those annotations. Also fixes a real gap the rule caught:
apps-vtex/utils/sitemap.ts's fetchImpl defaulted to raw fetch, same
unbounded-hang risk as the other call sites fixed earlier on this branch.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants