test_runner: avoid hanging on incomplete v8 frames#62704
Conversation
| // size, causing #processRawBuffer to make no progress and | ||
| // #drainRawBuffer to loop forever without the no-progress guard. | ||
| const reported = await collectReported([oversizedLengthHeader]); | ||
| assert(reported.every((event) => event.type === 'test:stdout')); |
There was a problem hiding this comment.
We can get a much more useful error output by using partialDeepStrictEqual here
| assert(reported.every((event) => event.type === 'test:stdout')); | |
| assert.partialDeepStrictEqual(reported, Array.from({ length: reported.length }, () => ({ type: 'test:stdout' })); |
| data: { | ||
| __proto__: null, | ||
| file: this.name, | ||
| message: TypedArrayPrototypeSubarray(bufferHead, 0, 1).toString('utf-8'), |
There was a problem hiding this comment.
If we're dealing with a single-byte UTF-8 char, we don't need the intermediate view I think
| message: TypedArrayPrototypeSubarray(bufferHead, 0, 1).toString('utf-8'), | |
| message: StringFromCharCode(bufferHead[0]), |
There was a problem hiding this comment.
it changes the output semantics here StringFromCharCode(0xff) gives "ÿ" but Buffer.from([0xff]).toString('utf8') gives "�" which matches the current stdout path so I’m keeping the UTF-8 decode
There was a problem hiding this comment.
String.fromCodePoint then – but I'm surprised we would get non-ASCII chars here
There was a problem hiding this comment.
Right, switched to StringFromCharCode the byte here is always the V8 version tag (0xFF)
Signed-off-by: Ali Hassan <ali-hassan27@outlook.com>
631f70a to
825b3e3
Compare
|
Can you add a proper PR description? |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #62704 +/- ##
==========================================
- Coverage 89.80% 89.80% -0.01%
==========================================
Files 699 699
Lines 216363 216417 +54
Branches 41370 41377 +7
==========================================
+ Hits 194309 194356 +47
- Misses 14140 14141 +1
- Partials 7914 7920 +6
🚀 New features to boost your workflow:
|
…-byte fallback path Signed-off-by: Ali Hassan <ali-hassan27@outlook.com>
|
@aduh95 can you please take a relook at this again? |
Signed-off-by: Ali Hassan <ali-hassan27@outlook.com> PR-URL: #62704 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Signed-off-by: Ali Hassan <ali-hassan27@outlook.com> PR-URL: #62704 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Signed-off-by: Ali Hassan <ali-hassan27@outlook.com> PR-URL: nodejs#62704 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Signed-off-by: Ali Hassan <ali-hassan27@outlook.com> PR-URL: #62704 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Signed-off-by: Ali Hassan <ali-hassan27@outlook.com> PR-URL: nodejs#62704 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Signed-off-by: Ali Hassan <ali-hassan27@outlook.com> PR-URL: #62704 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
This test synthesizes fake V8-serialized messages with the serialization format version hardcoded to 15 (0x0f). Electron bundles Chromium's V8, whose value-serializer format version is 16 (0x10), so the hardcoded headers are not recognized by the test runner's deserializer and the bytes fall through the UTF-8 stdout path (0xff -> U+FFFD), failing the assertions. The version difference is environmental and cannot be reconciled without re-patching the test on every V8 bump. The failing subtests were introduced upstream in nodejs/node#62704 (test_runner: avoid hanging on incomplete v8 frames).
This test synthesizes fake V8-serialized messages with the serialization format version hardcoded to 15 (0x0f). Electron bundles Chromium's V8, whose value-serializer format version is 16 (0x10), so the hardcoded headers are not recognized by the test runner's deserializer and the bytes fall through the UTF-8 stdout path (0xff -> U+FFFD), failing the assertions. The version difference is environmental and cannot be reconciled without re-patching the test on every V8 bump. The failing subtests were introduced upstream in nodejs/node#62704 (test_runner: avoid hanging on incomplete v8 frames).
This test synthesizes fake V8-serialized messages with the serialization format version hardcoded to 15 (0x0f). Electron bundles Chromium's V8, whose value-serializer format version is 16 (0x10), so the hardcoded headers are not recognized by the test runner's deserializer and the bytes fall through the UTF-8 stdout path (0xff -> U+FFFD), failing the assertions. The version difference is environmental and cannot be reconciled without re-patching the test on every V8 bump. The failing subtests were introduced upstream in nodejs/node#62704 (test_runner: avoid hanging on incomplete v8 frames).
This test synthesizes fake V8-serialized messages with the serialization format version hardcoded to 15 (0x0f). Electron bundles Chromium's V8, whose value-serializer format version is 16 (0x10), so the hardcoded headers are not recognized by the test runner's deserializer and the bytes fall through the UTF-8 stdout path (0xff -> U+FFFD), failing the assertions. The version difference is environmental and cannot be reconciled without re-patching the test on every V8 bump. The failing subtests were introduced upstream in nodejs/node#62704 (test_runner: avoid hanging on incomplete v8 frames).
* chore: bump node in DEPS to v24.17.0 * chore: update patches (trivial only) Co-Authored-By: GPT-5.3-Codex <copilot@github.com> * chore: bump node in DEPS to v24.18.0 * fix(patch): BoringSSL and OpenSSL incompatibilities Ref: nodejs/node@677ca7e76c9 Co-Authored-By: GPT-5.3-Codex <copilot@github.com> * fix(patch): crypto tests to run with BoringSSL Ref: nodejs/node@61b20f60a39 Co-Authored-By: GPT-5.3-Codex <copilot@github.com> * fix(patch): expose built-in electron module in ESM loader Ref: nodejs/node@69df688fff4 Co-Authored-By: GPT-5.3-Codex <copilot@github.com> * chore: update patches (trivial only) * fix(patch): adapt ESM tests for Electron Ref: nodejs/node@69df688fff4 Co-Authored-By: GPT-5.3-Codex <copilot@github.com> * chore: update filenames.auto.gni * test: update JWK error message assertion for unsupported algorithm The error thrown for an unsupported OKP (Ed448) JWK changed from 'Invalid JWK data' to the more specific 'Invalid JWK OKP key'. Ref: nodejs/node#62499 * test: disable parallel/test-runner-v8-deserializer This test synthesizes fake V8-serialized messages with the serialization format version hardcoded to 15 (0x0f). Electron bundles Chromium's V8, whose value-serializer format version is 16 (0x10), so the hardcoded headers are not recognized by the test runner's deserializer and the bytes fall through the UTF-8 stdout path (0xff -> U+FFFD), failing the assertions. The version difference is environmental and cannot be reconciled without re-patching the test on every V8 bump. The failing subtests were introduced upstream in nodejs/node#62704 (test_runner: avoid hanging on incomplete v8 frames). * docs: update node upgrade skill to reflect boringSSL updates * chore: update patch after rebase --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org> Co-authored-by: GPT-5.3-Codex <copilot@github.com>
This test synthesizes fake V8-serialized messages with the serialization format version hardcoded to 15 (0x0f). Electron bundles Chromium's V8, whose value-serializer format version is 16 (0x10), so the hardcoded headers are not recognized by the test runner's deserializer and the bytes fall through the UTF-8 stdout path (0xff -> U+FFFD), failing the assertions. The version difference is environmental and cannot be reconciled without re-patching the test on every V8 bump. The failing subtests were introduced upstream in nodejs/node#62704 (test_runner: avoid hanging on incomplete v8 frames).
* chore: bump node in DEPS to v24.17.0 Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> * chore: update patches (trivial only) Co-Authored-By: GPT-5.3-Codex <copilot@github.com> Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org> * chore: bump node in DEPS to v24.18.0 Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> * fix(patch): BoringSSL and OpenSSL incompatibilities Ref: nodejs/node@677ca7e76c9 Co-Authored-By: GPT-5.3-Codex <copilot@github.com> Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org> * fix(patch): crypto tests to run with BoringSSL Ref: nodejs/node@61b20f60a39 Co-Authored-By: GPT-5.3-Codex <copilot@github.com> Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org> * fix(patch): expose built-in electron module in ESM loader Ref: nodejs/node@69df688fff4 Co-Authored-By: GPT-5.3-Codex <copilot@github.com> Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org> * chore: update patches (trivial only) Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org> * fix(patch): adapt ESM tests for Electron Ref: nodejs/node@69df688fff4 Co-Authored-By: GPT-5.3-Codex <copilot@github.com> Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org> * chore: update filenames.auto.gni Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org> * test: update JWK error message assertion for unsupported algorithm The error thrown for an unsupported OKP (Ed448) JWK changed from 'Invalid JWK data' to the more specific 'Invalid JWK OKP key'. Ref: nodejs/node#62499 Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org> * test: disable parallel/test-runner-v8-deserializer This test synthesizes fake V8-serialized messages with the serialization format version hardcoded to 15 (0x0f). Electron bundles Chromium's V8, whose value-serializer format version is 16 (0x10), so the hardcoded headers are not recognized by the test runner's deserializer and the bytes fall through the UTF-8 stdout path (0xff -> U+FFFD), failing the assertions. The version difference is environmental and cannot be reconciled without re-patching the test on every V8 bump. The failing subtests were introduced upstream in nodejs/node#62704 (test_runner: avoid hanging on incomplete v8 frames). Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org> * docs: update node upgrade skill to reflect boringSSL updates Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org> * chore: update patch after rebase Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org> --------- Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
* chore: bump node in DEPS to v24.18.0 * fix(patch): crypto tests to run with BoringSSL Ref: nodejs/node@61b20f60a39 (cherry picked from commit 432172c) Co-Authored-By: GPT-5.3-Codex <copilot@github.com> * fix(patch): adapt ESM tests for Electron Ref: nodejs/node@69df688fff4 Co-Authored-By: GPT-5.3-Codex <copilot@github.com> (cherry picked from commit ce40736) * test: update JWK error message assertion for unsupported algorithm The error thrown for an unsupported OKP (Ed448) JWK changed from 'Invalid JWK data' to the more specific 'Invalid JWK OKP key'. Ref: nodejs/node#62499 (cherry picked from commit 5718143) * fix(patch): BoringSSL and OpenSSL incompatibilities Ref: nodejs/node@677ca7e76c9 Co-Authored-By: GPT-5.3-Codex <copilot@github.com> (cherry picked from commit 4d514ff) * chore: update patches (trivial only) * fix(patch): expose built-in electron module in ESM loader Ref: nodejs/node@69df688fff4 Co-Authored-By: GPT-5.3-Codex <copilot@github.com> * test: disable parallel/test-runner-v8-deserializer This test synthesizes fake V8-serialized messages with the serialization format version hardcoded to 15 (0x0f). Electron bundles Chromium's V8, whose value-serializer format version is 16 (0x10), so the hardcoded headers are not recognized by the test runner's deserializer and the bytes fall through the UTF-8 stdout path (0xff -> U+FFFD), failing the assertions. The version difference is environmental and cannot be reconciled without re-patching the test on every V8 bump. The failing subtests were introduced upstream in nodejs/node#62704 (test_runner: avoid hanging on incomplete v8 frames). * docs: update node upgrade skill to reflect boringSSL updates * chore: update filenames.auto.gni --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org> Co-authored-by: GPT-5.3-Codex <copilot@github.com>
* chore: bump node in DEPS to v24.18.0 * fix(patch): crypto tests to run with BoringSSL Ref: nodejs/node@61b20f60a39 (cherry picked from commit 432172c) Co-Authored-By: GPT-5.3-Codex <copilot@github.com> * fix(patch): adapt ESM tests for Electron Ref: nodejs/node@69df688fff4 Co-Authored-By: GPT-5.3-Codex <copilot@github.com> (cherry picked from commit ce40736) * test: update JWK error message assertion for unsupported algorithm The error thrown for an unsupported OKP (Ed448) JWK changed from 'Invalid JWK data' to the more specific 'Invalid JWK OKP key'. Ref: nodejs/node#62499 (cherry picked from commit 5718143) * fix(patch): BoringSSL and OpenSSL incompatibilities Ref: nodejs/node@677ca7e76c9 Co-Authored-By: GPT-5.3-Codex <copilot@github.com> (cherry picked from commit 4d514ff) * fix(patch): expose built-in electron module in ESM loader Ref: nodejs/node@69df688fff4 Co-Authored-By: GPT-5.3-Codex <copilot@github.com> * test: disable parallel/test-runner-v8-deserializer This test synthesizes fake V8-serialized messages with the serialization format version hardcoded to 15 (0x0f). Electron bundles Chromium's V8, whose value-serializer format version is 16 (0x10), so the hardcoded headers are not recognized by the test runner's deserializer and the bytes fall through the UTF-8 stdout path (0xff -> U+FFFD), failing the assertions. The version difference is environmental and cannot be reconciled without re-patching the test on every V8 bump. The failing subtests were introduced upstream in nodejs/node#62704 (test_runner: avoid hanging on incomplete v8 frames). * docs: update node upgrade skill to reflect boringSSL updates * chore: update patches (trivial only) * fix(patch): skip PQC crypto tests unsupported by BoringSSL Chromium 148's BoringSSL does not implement ML-KEM/ML-DSA/SLH-DSA, which upstream wired up for BoringSSL in nodejs/node#63255. Skip or disable the affected tests on BoringSSL for this Electron version. Ref: nodejs/node#63255 Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org> Co-authored-by: GPT-5.3-Codex <copilot@github.com> Co-authored-by: Claude <noreply@anthropic.com>
This test synthesizes fake V8-serialized messages with the serialization format version hardcoded to 15 (0x0f). Electron bundles Chromium's V8, whose value-serializer format version is 16 (0x10), so the hardcoded headers are not recognized by the test runner's deserializer and the bytes fall through the UTF-8 stdout path (0xff -> U+FFFD), failing the assertions. The version difference is environmental and cannot be reconciled without re-patching the test on every V8 bump. The failing subtests were introduced upstream in nodejs/node#62704 (test_runner: avoid hanging on incomplete v8 frames).
* chore: bump node in DEPS to v24.18.0 * fix(patch): crypto tests to run with BoringSSL Ref: nodejs/node@61b20f60a39 (cherry picked from commit 432172c) Co-Authored-By: GPT-5.3-Codex <copilot@github.com> * fix(patch): adapt ESM tests for Electron Ref: nodejs/node@69df688fff4 Co-Authored-By: GPT-5.3-Codex <copilot@github.com> (cherry picked from commit ce40736) * test: update JWK error message assertion for unsupported algorithm The error thrown for an unsupported OKP (Ed448) JWK changed from 'Invalid JWK data' to the more specific 'Invalid JWK OKP key'. Ref: nodejs/node#62499 (cherry picked from commit 5718143) * fix(patch): BoringSSL and OpenSSL incompatibilities Ref: nodejs/node@677ca7e76c9 Co-Authored-By: GPT-5.3-Codex <copilot@github.com> (cherry picked from commit 4d514ff) * fix(patch): expose built-in electron module in ESM loader Ref: nodejs/node@69df688fff4 Co-Authored-By: GPT-5.3-Codex <copilot@github.com> * test: disable parallel/test-runner-v8-deserializer This test synthesizes fake V8-serialized messages with the serialization format version hardcoded to 15 (0x0f). Electron bundles Chromium's V8, whose value-serializer format version is 16 (0x10), so the hardcoded headers are not recognized by the test runner's deserializer and the bytes fall through the UTF-8 stdout path (0xff -> U+FFFD), failing the assertions. The version difference is environmental and cannot be reconciled without re-patching the test on every V8 bump. The failing subtests were introduced upstream in nodejs/node#62704 (test_runner: avoid hanging on incomplete v8 frames). * docs: update node upgrade skill to reflect boringSSL updates * fix(patch): skip PQC crypto tests unsupported by BoringSSL Chromium 148's BoringSSL does not implement ML-KEM/ML-DSA/SLH-DSA, which upstream wired up for BoringSSL in nodejs/node#63255. Skip or disable the affected tests on BoringSSL for this Electron version. Ref: nodejs/node#63255 Co-Authored-By: Claude <noreply@anthropic.com> * chore: update patches (trivial only) * fix(patch): gate BoringSSL PQC surface on BORINGSSL_API_VERSION Chromium 146 ships BoringSSL API version 39, which exposes the ML-DSA and ML-KEM NIDs but not the ML-KEM EVP_PKEY_ALG accessors, the ML-KEM encapsulate/decapsulate APIs, or EVP_PKEY_CTX_set1_signature_context_string. Those only became available at API version 40 (Chromium 148). Gate the BoringSSL PQC/KEM/signature-context capability macros on the API version so the older BoringSSL builds with PQC reported as unsupported. Ref: nodejs/node#63255 Co-Authored-By: GPT-5.3-Codex <copilot@github.com> * fix(patch): skip PQC tests unsupported by 41-x-y BoringSSL Chromium 146's BoringSSL (API v39) lacks ML-KEM and ML-DSA support, so guard the new PQC keygen/webcrypto tests that assume BoringSSL provides PQC (as it does on newer Chromium). Drops the boringssl PQC-enable disjunct so these tests skip on this branch's BoringSSL. Ref: nodejs/node#63255 Co-Authored-By: GPT-5.3-Codex <copilot@github.com> --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org> Co-authored-by: GPT-5.3-Codex <copilot@github.com> Co-authored-by: Claude <noreply@anthropic.com>

The test runner could hang while draining child stdout when the buffered data started with bytes that matched the V8 serializer header (
0xff 0x0f) but did not contain a complete serialized frame.In that case
#processRawBuffer()made no progress, and#drainRawBuffer()kept calling it in a loop forever.This change makes the deserializer recover from that end-of-stream case by flushing the buffered bytes as stdout and continuing to resync. It also preserves the ordering of a trailing partial V8 header split across chunks.
Closes #62693
Assisted-by: Claude Opus