Skip to content

Commit

Permalink
Fix unhandled promise rejection handler after a .then
Browse files Browse the repository at this point in the history
If we have a promise `p1` that is rejected, and invoke `then` on it,
we create a new promise `p2` (as it should be, per the spec) and mark
`p1` as handled. However, if we call `.catch` on `p2`, we are _not_
marking `p1` as handled correctly since its status is "pending" and
not "rejected". This patch fixes it and adds some tests.

Fixes #1461
  • Loading branch information
andreabergia authored and gbrail committed May 3, 2024
1 parent 05c033d commit 15abaad
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 4 deletions.
15 changes: 11 additions & 4 deletions src/org/mozilla/javascript/NativePromise.java
Original file line number Diff line number Diff line change
Expand Up @@ -306,15 +306,19 @@ private Object then(
cx.enqueueMicrotask(() -> fulfillReaction.invoke(cx, scope, result));
} else {
assert (state == State.REJECTED);
if (!handled) {
cx.getUnhandledPromiseTracker().promiseHandled(this);
}
markHandled(cx);
cx.enqueueMicrotask(() -> rejectReaction.invoke(cx, scope, result));
}
handled = true;
return capability.promise;
}

private void markHandled(Context cx) {
if (!handled) {
cx.getUnhandledPromiseTracker().promiseHandled(this);
handled = true;
}
}

// Promise.prototype.catch
private static Object doCatch(Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
Object arg = (args.length > 0 ? args[0] : Undefined.instance);
Expand Down Expand Up @@ -446,6 +450,9 @@ private Object rejectPromise(Context cx, Scriptable scope, Object reason) {
for (Reaction r : reactions) {
cx.enqueueMicrotask(() -> r.invoke(cx, scope, reason));
}
if (!reactions.isEmpty()) {
markHandled(cx);
}
return Undefined.instance;
}

Expand Down
56 changes: 56 additions & 0 deletions testsrc/org/mozilla/javascript/tests/es6/UnhandledPromiseTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,46 @@ public void simpleRejectionHandled() {
assertTrue(Context.toBoolean(scope.get("caught", scope)));
}

@Test
public void rejectionHandledAfterThen() {
scope.put("caught", scope, Boolean.FALSE);
exec(
"new Promise((resolve, reject) => { reject(); })."
+ "then(() => {})."
+ "catch((e) => { caught = true; });\n");
assertTrue(cx.getUnhandledPromiseTracker().enumerate().isEmpty());
AtomicBoolean handled = new AtomicBoolean();
// We should actually never see anything in the tracker here
cx.getUnhandledPromiseTracker()
.process(
(Object o) -> {
assertFalse(handled.get());
handled.set(true);
});
assertFalse(handled.get());
assertTrue(Context.toBoolean(scope.get("caught", scope)));
}

@Test
public void thenRejectsButCatchHandles() {
scope.put("caught", scope, Boolean.FALSE);
exec(
"new Promise((resolve, reject) => { resolve(); })."
+ "then((e) => { return Promise.reject(); })."
+ "catch((e) => { caught = true; });\n");
assertTrue(cx.getUnhandledPromiseTracker().enumerate().isEmpty());
AtomicBoolean handled = new AtomicBoolean();
// We should actually never see anything in the tracker here
cx.getUnhandledPromiseTracker()
.process(
(Object o) -> {
assertFalse(handled.get());
handled.set(true);
});
assertFalse(handled.get());
assertTrue(Context.toBoolean(scope.get("caught", scope)));
}

@Test
public void rejectionObject() {
exec("new Promise((resolve, reject) => { reject('rejected'); });");
Expand Down Expand Up @@ -115,6 +155,22 @@ public void throwInThen() {
assertTrue(handled.get());
}

@Test
public void thenReturnsRejectedPromise() {
exec(
"new Promise((resolve, reject) => { resolve() }).then(() => { return Promise.reject('rejected'); });");
assertEquals(1, cx.getUnhandledPromiseTracker().enumerate().size());
AtomicBoolean handled = new AtomicBoolean();
cx.getUnhandledPromiseTracker()
.process(
(Object o) -> {
assertFalse(handled.get());
handled.set(true);
assertEquals("rejected", o);
});
assertTrue(handled.get());
}

private void exec(String script) {
cx.evaluateString(scope, "load('./testsrc/assert.js'); " + script, "test.js", 0, null);
}
Expand Down

0 comments on commit 15abaad

Please sign in to comment.