TextDecoder: throw ERR_STRING_TOO_LONG instead of returning an empty string - #37216
Open
robobun wants to merge 3 commits into
Open
TextDecoder: throw ERR_STRING_TOO_LONG instead of returning an empty string#37216robobun wants to merge 3 commits into
robobun wants to merge 3 commits into
Claude / Claude Code Review
completed
Aug 9, 2026 in 17m 22s
Code review found 1 important issue
Found 4 candidates, confirmed 3. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | test/js/web/encoding/text-decoder.test.js:965-973 |
2 GiB allocation test lacks low-memory guard |
| 🟡 Nit | test/js/web/encoding/text-decoder.test.js:966-972 |
Subprocess tests don't pipe/read stderr |
Annotations
Check failure on line 973 in test/js/web/encoding/text-decoder.test.js
claude / Claude Code Review
2 GiB allocation test lacks low-memory guard
The 2^31-byte test wraps `new Uint8Array(2 ** 31)` and `decode()` in one try/catch, so on runners where the 2 GiB reservation itself fails (Windows does not overcommit; Linux under memory pressure or `vm.overcommit_memory=2` refuses it) the catch prints `threw:undefined:Array buffer allocation failed` and the outer `toBe(expected)` fails as a false red. Every sibling 2^31 allocation test in the repo guards this — blob-oom.test.ts:158 / fs-oom.test.ts:62 / buffer.test.js:4642 gate on `skipIf(os.t
Check warning on line 972 in test/js/web/encoding/text-decoder.test.js
claude / Claude Code Review
Subprocess tests don't pipe/read stderr
Both new subprocess tests spawn without `stderr: "pipe"` and await only `[proc.stdout.text(), proc.exited]`, unlike the neighboring SharedArrayBuffer test in this file which pipes and reads all three. If the child aborts or is OOM-killed before reaching `console.log`, the diagnostic goes to the parent's inherited stderr and the assertion just shows `expected "" to be "threw:..."`. Add `stderr: "pipe"` and assert `{ stdout, stderr, exitCode }` in both `it` blocks (lines ~966-972 and ~977-985).
Loading