fix: replace Node.js pipe() with Web Streams pipeThrough() to fix NotReadableError flake#360
Conversation
…equestData
Node.js pipe() immediately puts the source stream into flowing mode via
process.nextTick(), which starts reading file-backed blobs (from fs.openAsBlob)
in the background. When nock intercepts fetch and replies without consuming
the request body, the async chain completes and test cleanup deletes the output
directory before the scheduled blob read fires. This causes an unhandled
NotReadableError that dd-trace misattributes to the test.each registration
context, making it appear as a flaky test failure.
Replacing with Web Streams pipeThrough(new CompressionStream('gzip')) keeps
the pipeline lazy: reading only starts when the output stream is consumed,
so no I/O races with file cleanup.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Uses the native Web Streams DecompressionStream to decompress the gzip output directly, eliminating the zlib import and the intermediate readFully helper. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Pull request overview
This PR addresses a flaky NotReadableError in unit tests by changing request-body gzip compression from Node.js stream.pipe() (which eagerly starts reading) to Web Streams pipeThrough() (lazy until consumed), preventing races with test cleanup when the request body is never read.
Changes:
- Updated
createRequestData()to return a WebReadableStreamand perform gzip compression viaCompressionStream('gzip')instead of NodeReadable.pipe(createGzip()). - Updated the sourcemaps sender unit test to decompress and read the request body via Web Streams (
DecompressionStream+Response.text()), matching the new stream type.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/core/src/helpers/request.ts | Switches request body serialization/compression to Web Streams and updates RequestData to use ReadableStream. |
| packages/plugins/error-tracking/src/sourcemaps/sender.test.ts | Adjusts test helper logic to read/decompress the new Web Stream payload. |
Comments suppressed due to low confidence (1)
packages/core/src/helpers/request.ts:89
Content-Encodingis being set to'multipart/form-data'whenzipis false. That value is not a valid content-encoding and can cause servers/proxies to mis-handle the request; theRequestalready provides the correctcontent-type(incl. boundary) viareq.headers. Consider only settingContent-Encoding: gzipwhenzipis true, and otherwise omitting the header entirely (or, if you intended to set content type, set/leavecontent-typeinstead).
const headers = {
'Content-Encoding': zip ? 'gzip' : 'multipart/form-data',
...defaultHeaders,
...Object.fromEntries(req.headers.entries()),
};
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Fixes a long-standing flaky test failure (
NotReadableError: The blob could not be read) in the unit test suite.Root Cause
The error is a race condition in
createRequestData()inpackages/core/src/helpers/request.ts.The chain of events:
getData()builds FormData withfs.openAsBlob()-backed blobs (lazy file references)createRequestData()serialized the form through aRequest, then used Node.jsReadable.fromWeb().pipe(createGzip())to compress itpipe()immediately calls.resume()on the source stream, scheduling actual blob reads viaprocess.nextTick()fetch()call and replies without consuming the request body — the async chain completes almost instantlyrm(rootDir)deletes the output directoryprocess.nextTick()fires, tries to read the now-deleted file-backed blob →NotReadableErrortest.eachregistration context, making it appear as a flaky failure on a random testFix
Replace
Readable.fromWeb().pipe(createGzip())with the Web StreamspipeThrough(new CompressionStream('gzip')). Web Streams are lazy: the pipeline only starts reading when the output stream is actually consumed (i.e., whenfetchreads the body). Since nock intercepts without reading the body, no blob I/O ever starts, eliminating the race.This also simplifies the code by removing the Node.js
streamandzlibimports in favor of the Web StreamsCompressionStream(available since Node.js 18).Changes
packages/core/src/helpers/request.ts: ReplaceGzip | ReadablewithReadableStream, useCompressionStreaminstead ofcreateGzip, remove Node.js stream/zlib importspackages/plugins/error-tracking/src/sourcemaps/sender.test.ts: UpdatereadFully()helper to use Web Streams API to match the newReadableStreamtypeTest plan
yarn test:unit packages/plugins/error-tracking— all 9 sender tests passyarn test:unit packages/core— all 11 request helper tests pass🤖 Generated with Claude Code