Skip to content

fix: bound maxDocumentBytes in bytes while the body is read - #18

Merged
dmccoystephenson merged 1 commit into
mainfrom
fix/bound-document-bytes-while-reading
Oct 8, 2026
Merged

dmccoystephenson merged 1 commit into
mainfrom
fix/bound-document-bytes-while-reading

Conversation

@dmccoystephenson

Copy link
Copy Markdown
Contributor

Summary

  • fetchMarkdown in js/src/client.ts no longer calls response.text() and then compares text.length to maxDocumentBytes. The body is read from response.body chunk by chunk, counted in bytes, and the read is cancelled and the request aborted (via the existing AbortController) as soon as the count passes the limit. The oversized remainder is therefore never buffered.
  • Decoding uses a streaming TextDecoder, so a multi-byte character split across chunks decodes correctly.
  • A response with no stream (an empty body, or a stand-in that only offers text()) falls back to text(), but is measured in UTF-8 bytes with TextEncoder, so the option has one meaning on both paths.
  • js/README.md's Configuration row for maxDocumentBytes now states that it is counted in bytes and that the read stops at the limit.
  • The existing test refuses a document larger than the limit rather than holding it in memory was renamed to refuses a document larger than the limit: its fake has no stream, so it never could observe memory, which was one of the points raised in maxDocumentBytes is checked against string length after the whole body is read, so it bounds neither bytes nor memory #16.

No Content-Length pre-check was added: the streaming count already stops within a chunk of the limit, and Content-Length describes the encoded transfer rather than the decoded body, so it would be a second, differently-measured limit.

Half touched: js/ only.

Nothing upstream is quoted back: every new failure path returns {status: 'unavailable'}, and the cancel rejection is swallowed rather than surfaced. No new throw is reachable from fetchMarkdown.

Test plan

  • CI js (18 | 20 | 22): typecheck, npm test, build
  • New: stops reading an oversized body at the limit instead of draining it — a 1000×64-byte stream against a 100-byte limit pulls fewer than 10 chunks, and the request signal is aborted
  • New: measures the limit in bytes, not in string length — 51 × é (102 bytes) is refused at a 100-byte limit, on both the stream and the text()-only path
  • New: decodes a character split across chunks when the body is within the limit
  • Regression evidence: with the fix reverted, the first two new tests are expected to fail (to be confirmed on CI — node is not available in the session sandbox, so the local anchor could not run)

Closes #16

Issues #5 and #6 were not picked up this cycle: every remaining step on both is an operator action outside this repository (registry credentials, the PyPI trusted publisher, release environments), as recorded on each issue.

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

🤖 Generated with Claude Code


drafted by Claude on behalf of Daniel Stephenson

The limit was applied as `text.length` after `response.text()` had
already buffered the whole body, so it bounded neither memory nor bytes.
The body is now read from its stream, counted in bytes as chunks arrive,
and the transfer is cancelled and aborted as soon as the count passes
the limit. A response with no stream still falls back to `text()`, but
is measured in UTF-8 bytes so the option means one thing either way.

Closes #16

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dmccoystephenson
dmccoystephenson force-pushed the fix/bound-document-bytes-while-reading branch from ebcc9ee to c209715 Compare October 8, 2026 09:19
@dmccoystephenson

Copy link
Copy Markdown
Contributor Author

Self-review rubric (head c209715):

  • Scope: PASS — three files, each required by maxDocumentBytes is checked against string length after the whole body is read, so it bounds neither bytes nor memory #16: js/src/client.ts (the fix), js/test/client.test.ts (regression tests plus the rename of a test whose name claimed a memory property its fake could not observe), js/README.md (the maxDocumentBytes row now states the unit and the stop-at-limit behavior).
  • Tests-new: PASS — readBoundedText is module-private; both of its paths (stream and text()-only) are exercised through fetchMarkdown by measures the limit in bytes, not in string length, and the stream path's overflow, abort and chunk-split decoding by the other two new tests.
  • Tests-fix: PASS — node is absent from the session sandbox, so fallback-ladder rung 1 was used. A commit ebcc9ee (TEMP: revert c209715 …) restored js/src/client.ts from main while keeping the new tests; CI run 37755811693 went red on all three js legs with exactly stops reading an oversized body at the limit instead of draining it and measures the limit in bytes, not in string length failing (2 failed | 77 passed). The branch was then reset to c209715 and force-pushed; CI is green again on that SHA. The third new test (chunk-split decoding) is a characterization guard for the new decoder and is expected to pass either way.
  • Sibling structure: PASS — no new files; the helper sits beside fetchMarkdown with a reason-stating comment in the module's style.
  • Sibling renames: PASS — no identifiers renamed; one test title was shortened.
  • Docs: PASS — js/README.md Configuration row updated; the MarkdownFetch comment ("an oversized body") and DEFAULT_MAX_DOCUMENT_BYTES's doc comment now describe what the code does; no export changed, so js/src/index.ts and the API list are unaffected.
  • Issue resolution: PASS — both defects named in maxDocumentBytes is checked against string length after the whole body is read, so it bounds neither bytes nor memory #16 are addressed: the body is no longer buffered past the limit, and the count is UTF-8 bytes rather than UTF-16 code units.
  • CI: PASS — all eight legs green on c209715; js (18) reports Tests 79 passed (79), 38 in client.test.ts.
  • No-leak (js): PASS — the new paths return only null or the decoded document; the reader.cancel() rejection is swallowed, and every failure still maps to {status: 'unavailable'}.
  • Result-not-exception: PASS — no throw added; a rejecting reader.read() (including a timeout abort mid-stream) lands in the existing catch.
  • Stdlib-only / No runtime dependency: PASS — python/ untouched; js/package.json and package-lock.json untouched. TextEncoder, TextDecoder and ReadableStream are globals on the Node 18 floor.
  • Export parity / Support-matrix parity / Sentinel intact: PASS — no exports, versions, or python/tests changes.

Judgment notes:

  • js/src/client.ts — controller.abort() after an overflow also covers a stream that ignores cancel(); the abort happens after the read has already returned, so it cannot turn into a rejected promise.
  • The text()-only fallback still buffers the whole body. On a real fetch Response it is reached only when body is null (an empty body), so it is not a memory path in practice; it exists for minimal stand-ins like the test fakes.
  • No Content-Length pre-check was added, deliberately: it measures the encoded transfer, not the decoded body, and would introduce a second limit measured differently.

Do-not-auto-merge check: the changed paths are js/README.md, js/src/client.ts and js/test/client.test.ts. None of them are on the list, and no file has more than 50 lines deleted.

This review comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).


drafted by Claude on behalf of Daniel Stephenson

@dmccoystephenson
dmccoystephenson merged commit 77725d4 into main Oct 8, 2026
32 checks passed
@dmccoystephenson
dmccoystephenson deleted the fix/bound-document-bytes-while-reading branch October 8, 2026 09:21
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.

maxDocumentBytes is checked against string length after the whole body is read, so it bounds neither bytes nor memory

1 participant