assert: prevent OOM when generating diff for large objects #61347
Conversation
Add a regression test for an issue where assert.strictEqual causes OOM when comparing objects with many converging paths to shared objects. The test creates an object graph similar to Mongoose documents and verifies that the assertion fails with an AssertionError rather than crashing.
nodejs-github-bot
added
needs-ci
Objects with many converging paths to shared objects can cause exponential growth in util.inspect output. When assert.strictEqual fails on such objects, the error message generation would OOM while trying to create a diff of the 100+ MB inspect strings. Add a 2MB limit to inspectValue() output. When truncation occurs, a marker is added and the error message indicates lines were skipped. The comparison itself is unaffected; only the error output is truncated.
AbdelrahmanHafez
changed the title
AbdelrahmanHafez
force-pushed
the
fix/assert-strictequal-oom
branch
from
413a099 to
3b6e7b3
Compare
AbdelrahmanHafez
force-pushed
the
fix/assert-strictequal-oom
branch
from
3b6e7b3 to
8905178
Compare
Co-authored-by: Aviv Keller <me@aviv.sh>
|
@avivkeller |
github-actions
bot
removed
the
request-ci
|
I've investigated the CI failures: GitHub Actions failures (coverage-linux, test-linux, test-tarball-linux, x86_64-linux): All terminated with exit code 143 due to runner shutdown signals, not test failures. The tests that completed before shutdown all passed. Jenkins CI failures (node-test-commit-linuxone, node-test-commit-plinux, node-test-linux-linked-*): These show "tests failed" but I cannot access the logs to verify if they're related to this PR or infrastructure issues. Could a maintainer please re-trigger CI or check the Jenkins logs? @avivkeller |
| // Maximum size for inspect output before truncation to prevent OOM. | ||
| // Objects with many converging paths can produce exponential growth in | ||
| // util.inspect output at high depths, leading to OOM during diff generation. | ||
| const kMaxInspectOutputLength = 2 * 1024 * 1024; // 2MB |
There was a problem hiding this comment.
Suggestion: split the string into chunks of 512kb and compare each chunk and combine the output. The output will not be perfect, as the combining process would sometimes be weird, while the smaller chunks should actually be much much faster to inspect in full.
There was a problem hiding this comment.
Good idea, will experiment with that and compare DX, and performance for any objects that are over 512kb. 👍
4 participants
Fixes #61346
Solution
Add a 2MB limit to
inspectValue()output inassertion_error.js. When truncation occurs:... [truncated]marker is added to the outputThe assertion logic is unaffected; only the error output is truncated.
Trade-offs
If both objects produce very large inspect output and happen to match exactly in the first 2MB but differ later, the diff would appear identical. However:
===failed)error.actualanderror.expectedprogrammaticallyAlternative approaches considered