assert: fix slow error message generation for minified code#62338
Open
Felipeness wants to merge 2 commits intonodejs:mainfrom
Open
assert: fix slow error message generation for minified code#62338Felipeness wants to merge 2 commits intonodejs:mainfrom
Felipeness wants to merge 2 commits intonodejs:mainfrom
Conversation
When assert.ok(falsy) is called in minified/bundled code (common in Lambda deployments via Webpack/Terser), the error message generation can take seconds or even minutes. The getFirstExpression function tokenizes the entire source line with acorn to extract the failing expression, but for minified files the source line is the entire file content (potentially megabytes of code on a single line). Fix by windowing the tokenization: when the source line exceeds 2048 characters, extract a ~1024 character window around the target column instead of tokenizing the entire line. The window uses semicolons as safe cut points to give acorn valid-ish input. A try-catch is added to gracefully handle cases where the windowed code starts mid-token. This preserves the detailed error message (including member access like assert.ok) while bounding tokenization work to a constant size regardless of file length. Fixes: nodejs#52677
nodejs-github-bot
added
errors
- Return undefined from catch block in getFirstExpression when acorn throws on malformed windowed code, instead of falling through and returning garbage from partially-tokenized input. - Reduce test fixture size from 100K variable declarations (~2.8MB) to ~3000 chars (just above kMaxSourceLineLength threshold). This still exercises the windowing path without stressing the filesystem. - Replace fixed 10s timeout as correctness assertion with elapsed time measurement via process.hrtime.bigint(). The test now asserts completion in under 10 seconds, which is more robust on slow CI.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When
assert.ok(falsy)is called in minified/bundled code (common in Lambda deployments via Webpack/Terser), the error message generation can take seconds to minutes. ThegetFirstExpressionfunction inlib/internal/errors/error_source.jstokenizes the entire source line with acorn to extract the failing expression, but for minified single-line files the source line is the entire file (potentially megabytes).This PR fixes the performance issue by windowing the tokenization:
assert.ok) is preservedBenchmark results (tested manually)
Test plan
test/parallel/test-assert-long-line-perf.jswith two tests:assert.okdoes not hang on a large synthetic minified file (100K declarations + assert)ok(false))test-assert-first-line.jstest (which testsassert-long-line.js, a 9.4KB single-line fixture) still produces the correct error message with windowing enabledassert.ok(false),assert['ok'](false), and plain;-padded assert callsFixes: #52677
Jira: N/A (open source contribution)