fix(reporter): fail perfetto writes when the output stream errors - #42715
Sebastien Tardif (SebTardif) wants to merge 4 commits into
Conversation
A failed write after backpressure waited forever for drain. Reject that wait on error and settle the close promise so the reporter exits. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Listen for file-stream errors while waiting on gzip drain. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
| 'a.test.ts': ` | ||
| import { test } from '@playwright/test'; | ||
| test('one', async () => { | ||
| test.info().annotations.push({ type: 'blob', description: ${JSON.stringify('x'.repeat(64 * 1024))} }); |
There was a problem hiding this comment.
this still passes without the fix because the repeated x compresses too well to reproduce the hang
There was a problem hiding this comment.
this still passes without the fix because the repeated
xcompresses too well to reproduce the hang
The test now writes incompressible data to a fifo and closes the reader so the dest stream errors. Without the dest listener the inner run exits 0. With it, exit is non-zero. f98bd0941
There was a problem hiding this comment.
this still passes without the fix because the repeated
xcompresses too well to reproduce the hang
The test now writes a 256KB incompressible annotation to a directory named trace.json or trace.json.gz. On main the inner run exits 0. With the once(drain) wait it is non-zero. No fifo.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Repeated x compresses too well, so /dev/full never errors. Write incompressible data to a fifo and close the reader so the dest stream errors. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
This can be a lot smaller. Three notes: The fix. So the whole change is: import { once } from 'events';
...
// pipe() only unpipes on a destination error, so the gzip stream would never
// emit 'drain' or 'error' again. Destroy it to wake up the pending write.
if (gzip)
fileStream.on('error', error => gzip.destroy(error));
...
if (!this._stream.write(chunk))
await once(this._stream, 'drain'); // Rejects if 'error' is emitted first.I ran the perfetto spec (including your new test) plus plain/gzip × directory/fifo scenarios with this; all green. The symptom. This isn't actually a hang. Once the drain promise is stuck there are no live handles, so Node exits with code 0, and the runner computes the exit code only after reporters finish The test. |
Wait with events.once(stream, 'drain') so an error unblocks the write. Forward file-stream errors through gzip.destroy so gzip waits reject. Test both .json and .json.gz against a directory plus a 256KB payload. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
Sebastien Tardif (SebTardif)
changed the title
Yes. The tests write a 256KB incompressible annotation to a directory named The description now says the stuck drain is a silent exit 0, not a hang. |
Test results for "tests 1"2 failed 7 flaky52007 passed, 1247 skipped Merge workflow run. |
Test results for "MCP"1 failed 8628 passed, 1446 skipped Merge workflow run. |
3 participants
Summary
outputFileafter backpressure, instead of exiting 0 with no report.Problem
On main,
ChunkWriter.writewaits only for'drain'. Ifwrite()returns false and the stream then errors (for exampleoutputFileis a directory), the drain promise never settles. There are no live handles, so Node exits 0. The runner computes the process exit code only after reporters finishonEnd, so a failing test plus a badoutputFilereports success and writes nothing.Change
Wait with
events.once(stream, 'drain'), which rejects if'error'fires first. For gzip,pipe()only unpipes on a destination error, so the file stream error is forwarded withgzip.destroy(error).Validation
tests/playwright-test/reporter-perfetto.spec.ts: directoryoutputFileplus a 256KB incompressible annotation, for both.jsonand.json.gz. On main the inner run exits 0. With this change it is non-zero.