Set module internal promise rejection as handled when appropriate#1554
Open
xim wants to merge 1 commit into
Open
Set module internal promise rejection as handled when appropriate#1554xim wants to merge 1 commit into
xim wants to merge 1 commit into
Conversation
bnoordhuis
reviewed
Jul 2, 2026
Contributor
Can we have a JS test for this? |
xim
force-pushed
the
rejection-unhandled-notification-dedup
branch
from
18b8212 to
badcfb6
Compare
Author
I'm having a stab at it now Edit: Done! |
xim
force-pushed
the
rejection-unhandled-notification-dedup
branch
from
efe13bc to
9faff3f
Compare
Before:
A module body is run as an async function. js_execute_sync_module calls
that function. It finds its result promise already rejected, reads the
result, and frees it without attaching a JS handler. The rejection fires
a unhandled notification. That notification is never balanced by a
is_handled=true notification. The internal promise rejection surfaces as
an unhandled rejection in addition to the module's evaluation promise.
Additional symptoms:
A dynamic import() whose rejection is fully handled by the caller still
leaked a unhandled rejection: the internal body promise stayed orphaned.
Without the fix a `import('m').then(ok, onerr)` of a throwing module
results in a unhandled rejection.
Fix:
Mark the consumed promise handled, emitting the balancing notification
with is_handled=true so the host can handle the rejection correctly.
With the fix, a `import('m').then(ok, onerr)` nets zero false positives.
Added both JS test and api-test regression tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Ben Noordhuis <info@bnoordhuis.nl>
xim
force-pushed
the
rejection-unhandled-notification-dedup
branch
from
9faff3f to
cd9a045
Compare
xim
added a commit
to xim/quickjs
that referenced
this pull request
Jul 2, 2026
Before:
A module body is run as an async function. js_execute_sync_module calls
that function. It finds its result promise already rejected, reads the
result, and frees it without attaching a JS handler. The rejection fires
a unhandled notification. That notification is never balanced by a
is_handled=true notification. The internal promise rejection surfaces as
an unhandled rejection in addition to the module's evaluation promise.
Additional symptoms:
A dynamic import() whose rejection is fully handled by the caller still
leaked a unhandled rejection: the internal body promise stayed orphaned.
Without the fix a `import('m').then(ok, onerr)` of a throwing module
results in a unhandled rejection.
Fix:
Mark the consumed promise handled, emitting the balancing notification
with is_handled=true so the host can handle the rejection correctly.
With the fix, a `import('m').then(ok, onerr)` nets zero false positives.
Added a regression test in tests/test_std.js covering the dynamic import
case (this repo has no api-test harness).
Backported from quickjs-ng PR:
quickjs-ng/quickjs#1554
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
3 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.
Before:
A module body is run as an async function. js_execute_sync_module calls that function. It finds its result promise already rejected, reads the result, and frees it without attaching a JS handler. The rejection fires a unhandled notification. That notification is never balanced by a is_handled=true notification. The internal promise rejection surfaces as an unhandled rejection in addition to the module's evaluation promise.
Additional symptoms:
A dynamic import() whose rejection is fully handled by the caller still leaked a unhandled rejection: the internal body promise stayed orphaned. Without the fix a
import('m').then(ok, onerr)of a throwing module results in a unhandled rejection.Fix:
Mark the consumed promise handled, emitting the balancing notification with is_handled=true so the host can handle the rejection correctly.
With the fix, a
import('m').then(ok, onerr)nets zero false positives.Added both JS test and api-test regression tests.