Skip to content

fix(cli): keep blog adapter timeout active through body reads - #5286

Merged
cixzhang merged 1 commit into
facebook:mainfrom
HelloOjasMutreja:fix/cli-blog-adapter-timeout-and-size-cap
Sep 3, 2026
Merged

fix(cli): keep blog adapter timeout active through body reads#5286
cixzhang merged 1 commit into
facebook:mainfrom
HelloOjasMutreja:fix/cli-blog-adapter-timeout-and-size-cap

Conversation

@HelloOjasMutreja

Copy link
Copy Markdown
Contributor

Fixes #5249.

Problem

The shared CLI blog adapter (packages/cli/api/blog/_adapter.mjs, used by both blog.list and blog.detail) had two related issues in fetchText:

  1. It cleared its 15-second abort timer as soon as fetch() returned response headers, before reading the body. A response whose headers arrive promptly but whose body stream stalls afterward has no timeout protecting it, and the call hangs indefinitely.
  2. It read the body via res.text(), which buffers the entire response into memory before the code ever checks the 5 MB limit. The limit rejects an oversized response only after it has already been fully read, so it doesn't actually cap how much data gets buffered.

Fix

  • The abort timer is now cleared in a finally block after the read completes (success or failure), so it stays active through body consumption, not just through the initial fetch() call.
  • The body is read via res.body's stream where available (getReader(), decoding chunk by chunk), checking the running decoded size after each chunk and cancelling the read as soon as it exceeds 5 MB, instead of buffering the full response first. Falls back to res.text() when a runtime or test stub doesn't expose a streamable body (rare in practice; every real fetch() response has one), still enforcing the cap after the fact in that case.

Verification

Added two regression tests matching the issue's own reproduction:

  • Timeout stays active through body consumption: a stubbed fetch resolves immediately but its response body never closes. Using fake timers, advancing past the 15s timeout causes the call to reject with ERR_FETCH_FAILED rather than hang. The stub's stream reacts to the request's AbortSignal the way a real fetch response body does (rejects pending reads on abort), so this faithfully exercises the same mechanism a real network response uses.
  • Size cap stops the read early: a stubbed response streams 5 chunks of 2 MB each (10 MB total) against the 5 MB cap. The call rejects with ERR_FETCH_FAILED, and the stub's pulled-chunk counter confirms fewer than all 5 chunks were consumed, proving the read stopped early rather than buffering the whole body before checking.

Both new tests fail against the pre-fix implementation (verified by temporarily reverting just the implementation file and re-running) and pass with the fix. Full blog test suite (22 tests across _adapter, blog, blog.detail, blog.list) passes. Lint required adding ReadableStream/DOMException to the CLI's ESLint global allowlist (needed by the new streaming-body test coverage; both are standard globals already implicitly relied on via fetch/AbortController). Typecheck (tsconfig.strict.json) shows no new errors; the pre-existing failures it reports are all in unrelated template/theme assets.

@vercel

vercel Bot commented Aug 21, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
astryx Ready Ready Preview Aug 31, 2026 9:32am

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Aug 21, 2026
@github-actions github-actions Bot added community Authored by a community contributor (not on the eng/design team) needs:code-review High-risk change (new package/component/API) — needs human code review before merge labels Aug 21, 2026
@HelloOjasMutreja
HelloOjasMutreja marked this pull request as ready for review August 21, 2026 08:47
@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

PR Analysis Report

Preview availability: Storybook and Sandbox were not published for this CI run.

No new or modified components detected.

Bundle Size Summary

No component packages changed.

Accessibility Audit

Status: No accessibility violations detected.


Generated by PR Enrichment workflow | View full report

github-actions Bot added a commit that referenced this pull request Aug 21, 2026
The shared blog adapter's 15-second abort timer was cleared as soon
as fetch returned response headers, leaving the later body read
unbounded in time. It also buffered the entire response into memory
via res.text() before checking the 5 MB limit, so the limit never
actually capped how much was read.

The timer now stays active through body consumption (cleared in a
finally block after the read completes or fails). The body is read
via its stream where available, checking decoded size after each
chunk and aborting as soon as it exceeds the limit, rather than
buffering the full response first. Falls back to res.text() when a
runtime or test stub doesn't expose a streamable body, still
enforcing the cap after the fact in that case.

Also adds ReadableStream and DOMException to the CLI's ESLint global
allowlist, needed for the new streaming-body test coverage.

@cixzhang cixzhang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is good. The stalled body now times out and the oversized stream stops early without changing the CLI error contract.

[Reviewed by Robohands]

@github-actions github-actions Bot removed the needs:code-review High-risk change (new package/component/API) — needs human code review before merge label Sep 3, 2026
@cixzhang
cixzhang merged commit a1977de into facebook:main Sep 3, 2026
28 checks passed
josephfarina pushed a commit that referenced this pull request Sep 4, 2026
The shared blog adapter's 15-second abort timer was cleared as soon
as fetch returned response headers, leaving the later body read
unbounded in time. It also buffered the entire response into memory
via res.text() before checking the 5 MB limit, so the limit never
actually capped how much was read.

The timer now stays active through body consumption (cleared in a
finally block after the read completes or fails). The body is read
via its stream where available, checking decoded size after each
chunk and aborting as soon as it exceeds the limit, rather than
buffering the full response first. Falls back to res.text() when a
runtime or test stub doesn't expose a streamable body, still
enforcing the cap after the fact in that case.

Also adds ReadableStream and DOMException to the CLI's ESLint global
allowlist, needed for the new streaming-body test coverage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot. community Authored by a community contributor (not on the eng/design team)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] CLI blog body reads bypass the timeout and size cap

2 participants