Promise.withResolvers() returned object had resolve and reject functions swapped (#1983)

tomatosalat0 · web-flow · commit 1b0d5f8a643a · 2024-10-24T08:37:18.000+03:00
diff --git a/Jint.Tests/Runtime/PromiseTests.cs b/Jint.Tests/Runtime/PromiseTests.cs
@@ -497,4 +497,56 @@ public void ManualPromise_HasCorrectStackTrace()
 
         Assert.Equal("at <anonymous>:1:56", logMessage?.Trim());
     }
+
+    [Fact]
+    public void WithResolvers_calling_resolve_resolves_promise()
+    {
+        // Arrange
+        using var engine = new Engine();
+        List<string> logMessages = new();
+        engine.SetValue("log", logMessages.Add);
+
+        // Act
+        engine.Execute("""
+                       const p = Promise.withResolvers();
+                       const next = p.promise
+                           .then(() => log('resolved'))
+                           .catch(() => log('rejected'));
+                           
+                       log('start');
+                       p.resolve();
+                       log('end');
+                       """);
+        engine.RunAvailableContinuations();
+
+        // Assert
+        List<string> expected = new() { "start", "end", "resolved" };
+        Assert.Equal(expected, logMessages);
+    }
+
+    [Fact]
+    public void WithResolvers_calling_reject_rejects_promise()
+    {
+        // Arrange
+        using var engine = new Engine();
+        List<string> logMessages = new();
+        engine.SetValue("log", logMessages.Add);
+
+        // Act
+        engine.Execute("""
+                       const p = Promise.withResolvers();
+                       const next = p.promise
+                           .then(() => log('resolved'))
+                           .catch(() => log('rejected'));
+                           
+                       log('start');
+                       p.reject();
+                       log('end');
+                       """);
+        engine.RunAvailableContinuations();
+
+        // Assert
+        List<string> expected = new() { "start", "end", "rejected" };
+        Assert.Equal(expected, logMessages);
+    }
 }
diff --git a/Jint/Native/Promise/PromiseConstructor.cs b/Jint/Native/Promise/PromiseConstructor.cs
@@ -13,9 +13,8 @@ internal sealed record PromiseCapability(
     JsValue PromiseInstance,
     ICallable Resolve,
     ICallable Reject,
-    JsValue RejectObj,
-    JsValue ResolveObj
-);
+    JsValue ResolveObj,
+    JsValue RejectObj);
 
 internal sealed class PromiseConstructor : Constructor
 {
@@ -139,11 +138,11 @@ private JsValue PromiseResolve(JsValue thisObject, JsValue x)
             }
         }
 
-        var (instance, resolve, _, _, _) = NewPromiseCapability(_engine, thisObject);
+        var capability = NewPromiseCapability(_engine, thisObject);
 
-        resolve.Call(Undefined, new[] { x });
+        capability.Resolve.Call(Undefined, new[] { x });
 
-        return instance;
+        return capability.PromiseInstance;
     }
 
     /// <summary>
@@ -163,11 +162,11 @@ private JsValue Reject(JsValue thisObject, JsValue[] arguments)
 
         var r = arguments.At(0);
 
-        var (instance, _, reject, _, _) = NewPromiseCapability(_engine, thisObject);
+        var capability = NewPromiseCapability(_engine, thisObject);
 
-        reject.Call(Undefined, new[] { r });
+        capability.Reject.Call(Undefined, new[] { r });
 
-        return instance;
+        return capability.PromiseInstance;
     }
 
     /// <summary>
@@ -255,8 +254,6 @@ private JsValue All(JsValue thisObject, JsValue[] arguments)
         if (!TryGetPromiseCapabilityAndIterator(thisObject, arguments, "Promise.all", out var capability, out var promiseResolve, out var iterator))
             return capability.PromiseInstance;
 
-        var (resultingPromise, resolve, reject, _, rejectObj) = capability;
-
         var results = new List<JsValue>();
         bool doneIterating = false;
 
@@ -269,7 +266,7 @@ void ResolveIfFinished()
             if (results.TrueForAll(static x => x is not null) && doneIterating)
             {
                 var array = _realm.Intrinsics.Array.ConstructFast(results);
-                resolve.Call(Undefined, new JsValue[] { array });
+                capability.Resolve.Call(Undefined, new JsValue[] { array });
             }
         }
 
@@ -296,8 +293,8 @@ void ResolveIfFinished()
                 }
                 catch (JavaScriptException e)
                 {
-                    reject.Call(Undefined, new[] { e.Error });
-                    return resultingPromise;
+                    capability.Reject.Call(Undefined, new[] { e.Error });
+                    return capability.PromiseInstance;
                 }
 
                 // note that null here is important
@@ -325,7 +322,7 @@ void ResolveIfFinished()
                             return Undefined;
                         }, 1, PropertyFlag.Configurable);
 
-                    thenFunc.Call(item, new JsValue[] { onSuccess, rejectObj });
+                    thenFunc.Call(item, new JsValue[] { onSuccess, capability.RejectObj });
                 }
                 else
                 {
@@ -338,11 +335,11 @@ void ResolveIfFinished()
         catch (JavaScriptException e)
         {
             iterator.Close(CompletionType.Throw);
-            reject.Call(Undefined, new[] { e.Error });
-            return resultingPromise;
+            capability.Reject.Call(Undefined, new[] { e.Error });
+            return capability.PromiseInstance;
         }
 
-        return resultingPromise;
+        return capability.PromiseInstance;
     }
 
     // https://tc39.es/ecma262/#sec-promise.allsettled
@@ -351,8 +348,6 @@ private JsValue AllSettled(JsValue thisObject, JsValue[] arguments)
         if (!TryGetPromiseCapabilityAndIterator(thisObject, arguments, "Promise.allSettled", out var capability, out var promiseResolve, out var iterator))
             return capability.PromiseInstance;
 
-        var (resultingPromise, resolve, reject, _, rejectObj) = capability;
-
         var results = new List<JsValue>();
         bool doneIterating = false;
 
@@ -365,7 +360,7 @@ void ResolveIfFinished()
             if (results.TrueForAll(static x => x is not null) && doneIterating)
             {
                 var array = _realm.Intrinsics.Array.ConstructFast(results);
-                resolve.Call(Undefined, new JsValue[] { array });
+                capability.Resolve.Call(Undefined, new JsValue[] { array });
             }
         }
 
@@ -392,8 +387,8 @@ void ResolveIfFinished()
                 }
                 catch (JavaScriptException e)
                 {
-                    reject.Call(Undefined, new[] { e.Error });
-                    return resultingPromise;
+                    capability.Reject.Call(Undefined, new[] { e.Error });
+                    return capability.PromiseInstance;
                 }
 
                 // note that null here is important
@@ -456,11 +451,11 @@ void ResolveIfFinished()
         catch (JavaScriptException e)
         {
             iterator.Close(CompletionType.Throw);
-            reject.Call(Undefined, new[] { e.Error });
-            return resultingPromise;
+            capability.Reject.Call(Undefined, new[] { e.Error });
+            return capability.PromiseInstance;
         }
 
-        return resultingPromise;
+        return capability.PromiseInstance;
     }
 
     // https://tc39.es/ecma262/#sec-promise.any
@@ -471,8 +466,6 @@ private JsValue Any(JsValue thisObject, JsValue[] arguments)
             return capability.PromiseInstance;
         }
 
-        var (resultingPromise, resolve, reject, resolveObj, _) = capability;
-
         var errors = new List<JsValue>();
         var doneIterating = false;
 
@@ -487,7 +480,7 @@ void RejectIfAllRejected()
             {
                 var array = _realm.Intrinsics.Array.ConstructFast(errors);
 
-                reject.Call(Undefined, new JsValue[] { Construct(_realm.Intrinsics.AggregateError, new JsValue[] { array }) });
+                capability.Reject.Call(Undefined, new JsValue[] { Construct(_realm.Intrinsics.AggregateError, new JsValue[] { array }) });
             }
         }
 
@@ -547,7 +540,7 @@ void RejectIfAllRejected()
                             return Undefined;
                         }, 1, PropertyFlag.Configurable);
 
-                    thenFunc.Call(item, new JsValue[] { resolveObj, onError });
+                    thenFunc.Call(item, new JsValue[] { capability.ResolveObj, onError });
                 }
                 else
                 {
@@ -560,11 +553,11 @@ void RejectIfAllRejected()
         catch (JavaScriptException e)
         {
             iterator.Close(CompletionType.Throw);
-            reject.Call(Undefined, new[] { e.Error });
-            return resultingPromise;
+            capability.Reject.Call(Undefined, new[] { e.Error });
+            return capability.PromiseInstance;
         }
 
-        return resultingPromise;
+        return capability.PromiseInstance;
     }
 
     // https://tc39.es/ecma262/#sec-promise.race
@@ -573,9 +566,6 @@ private JsValue Race(JsValue thisObject, JsValue[] arguments)
         if (!TryGetPromiseCapabilityAndIterator(thisObject, arguments, "Promise.race", out var capability, out var promiseResolve, out var iterator))
             return capability.PromiseInstance;
 
-        var (resultingPromise, resolve, reject, _, rejectObj) = capability;
-
-
         // 7. Let result be PerformPromiseRace(iteratorRecord, C, promiseCapability, promiseResolve).
         // https://tc39.es/ecma262/#sec-performpromiserace
         try
@@ -594,16 +584,16 @@ private JsValue Race(JsValue thisObject, JsValue[] arguments)
                 }
                 catch (JavaScriptException e)
                 {
-                    reject.Call(Undefined, new[] { e.Error });
-                    return resultingPromise;
+                    capability.Reject.Call(Undefined, new[] { e.Error });
+                    return capability.PromiseInstance;
                 }
 
                 // h. Let nextPromise be ? Call(promiseResolve, constructor, « nextValue »).
                 var nextPromise = promiseResolve.Call(thisObject, new JsValue[] { nextValue });
 
                 // i. Perform ? Invoke(nextPromise, "then", « resultCapability.[[Resolve]], resultCapability.[[Reject]] »).
 
-                _engine.Invoke(nextPromise, "then", new[] { (JsValue) resolve, rejectObj });
+                _engine.Invoke(nextPromise, "then", new[] { (JsValue) capability.Resolve, capability.RejectObj });
             } while (true);
         }
         catch (JavaScriptException e)
@@ -612,13 +602,13 @@ private JsValue Race(JsValue thisObject, JsValue[] arguments)
             // a. If iteratorRecord.[[Done]] is false, set result to IteratorClose(iteratorRecord, result).
             //     b. IfAbruptRejectPromise(result, promiseCapability).
             iterator.Close(CompletionType.Throw);
-            reject.Call(Undefined, new[] { e.Error });
-            return resultingPromise;
+            capability.Reject.Call(Undefined, new[] { e.Error });
+            return capability.PromiseInstance;
         }
 
         // 9. Return Completion(result).
         // Note that PerformPromiseRace returns a Promise instance in success case
-        return resultingPromise;
+        return capability.PromiseInstance;
     }
 
 
@@ -715,6 +705,11 @@ JsValue Executor(JsValue thisObject, JsValue[] arguments)
             ExceptionHelper.ThrowTypeError(engine.Realm, "reject is not a function");
         }
 
-        return new PromiseCapability(instance, resolve, reject, resolveArg, rejectArg);
+        return new PromiseCapability(
+            PromiseInstance: instance,
+            Resolve: resolve,
+            Reject: reject,
+            RejectObj: rejectArg,
+            ResolveObj: resolveArg);
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -13,9 +13,8 @@ internal sealed record PromiseCapability(`
`13`	`13`	`JsValue PromiseInstance,`
`14`	`14`	`ICallable Resolve,`
`15`	`15`	`ICallable Reject,`
`16`		`- JsValue RejectObj,`
`17`		`- JsValue ResolveObj`
`18`		`-);`
	`16`	`+ JsValue ResolveObj,`
	`17`	`+ JsValue RejectObj);`
`19`	`18`
`20`	`19`	`internal sealed class PromiseConstructor : Constructor`
`21`	`20`	`{`
`@@ -139,11 +138,11 @@ private JsValue PromiseResolve(JsValue thisObject, JsValue x)`
`139`	`138`	`}`
`140`	`139`	`}`
`141`	`140`
`142`		`- var (instance, resolve, _, _, _) = NewPromiseCapability(_engine, thisObject);`
	`141`	`+ var capability = NewPromiseCapability(_engine, thisObject);`
`143`	`142`
`144`		`- resolve.Call(Undefined, new[] { x });`
	`143`	`+ capability.Resolve.Call(Undefined, new[] { x });`
`145`	`144`
`146`		`- return instance;`
	`145`	`+ return capability.PromiseInstance;`
`147`	`146`	`}`
`148`	`147`
`149`	`148`	`/// <summary>`
`@@ -163,11 +162,11 @@ private JsValue Reject(JsValue thisObject, JsValue[] arguments)`
`163`	`162`
`164`	`163`	`var r = arguments.At(0);`
`165`	`164`
`166`		`- var (instance, _, reject, _, _) = NewPromiseCapability(_engine, thisObject);`
	`165`	`+ var capability = NewPromiseCapability(_engine, thisObject);`
`167`	`166`
`168`		`- reject.Call(Undefined, new[] { r });`
	`167`	`+ capability.Reject.Call(Undefined, new[] { r });`
`169`	`168`
`170`		`- return instance;`
	`169`	`+ return capability.PromiseInstance;`
`171`	`170`	`}`
`172`	`171`
`173`	`172`	`/// <summary>`
`@@ -255,8 +254,6 @@ private JsValue All(JsValue thisObject, JsValue[] arguments)`
`255`	`254`	`if (!TryGetPromiseCapabilityAndIterator(thisObject, arguments, "Promise.all", out var capability, out var promiseResolve, out var iterator))`
`256`	`255`	`return capability.PromiseInstance;`
`257`	`256`
`258`		`- var (resultingPromise, resolve, reject, _, rejectObj) = capability;`
`259`		`-`
`260`	`257`	`var results = new List<JsValue>();`
`261`	`258`	`bool doneIterating = false;`
`262`	`259`
`@@ -269,7 +266,7 @@ void ResolveIfFinished()`
`269`	`266`	`if (results.TrueForAll(static x => x is not null) && doneIterating)`
`270`	`267`	`{`
`271`	`268`	`var array = _realm.Intrinsics.Array.ConstructFast(results);`
`272`		`- resolve.Call(Undefined, new JsValue[] { array });`
	`269`	`+ capability.Resolve.Call(Undefined, new JsValue[] { array });`
`273`	`270`	`}`
`274`	`271`	`}`
`275`	`272`
`@@ -296,8 +293,8 @@ void ResolveIfFinished()`
`296`	`293`	`}`
`297`	`294`	`catch (JavaScriptException e)`
`298`	`295`	`{`
`299`		`- reject.Call(Undefined, new[] { e.Error });`
`300`		`- return resultingPromise;`
	`296`	`+ capability.Reject.Call(Undefined, new[] { e.Error });`
	`297`	`+ return capability.PromiseInstance;`
`301`	`298`	`}`
`302`	`299`
`303`	`300`	`// note that null here is important`
`@@ -325,7 +322,7 @@ void ResolveIfFinished()`
`325`	`322`	`return Undefined;`
`326`	`323`	`}, 1, PropertyFlag.Configurable);`
`327`	`324`
`328`		`- thenFunc.Call(item, new JsValue[] { onSuccess, rejectObj });`
	`325`	`+ thenFunc.Call(item, new JsValue[] { onSuccess, capability.RejectObj });`
`329`	`326`	`}`
`330`	`327`	`else`
`331`	`328`	`{`
`@@ -338,11 +335,11 @@ void ResolveIfFinished()`
`338`	`335`	`catch (JavaScriptException e)`
`339`	`336`	`{`
`340`	`337`	`iterator.Close(CompletionType.Throw);`
`341`		`- reject.Call(Undefined, new[] { e.Error });`
`342`		`- return resultingPromise;`
	`338`	`+ capability.Reject.Call(Undefined, new[] { e.Error });`
	`339`	`+ return capability.PromiseInstance;`
`343`	`340`	`}`
`344`	`341`
`345`		`- return resultingPromise;`
	`342`	`+ return capability.PromiseInstance;`
`346`	`343`	`}`
`347`	`344`
`348`	`345`	`// https://tc39.es/ecma262/#sec-promise.allsettled`
`@@ -351,8 +348,6 @@ private JsValue AllSettled(JsValue thisObject, JsValue[] arguments)`
`351`	`348`	`if (!TryGetPromiseCapabilityAndIterator(thisObject, arguments, "Promise.allSettled", out var capability, out var promiseResolve, out var iterator))`
`352`	`349`	`return capability.PromiseInstance;`
`353`	`350`
`354`		`- var (resultingPromise, resolve, reject, _, rejectObj) = capability;`
`355`		`-`
`356`	`351`	`var results = new List<JsValue>();`
`357`	`352`	`bool doneIterating = false;`
`358`	`353`
`@@ -365,7 +360,7 @@ void ResolveIfFinished()`
`365`	`360`	`if (results.TrueForAll(static x => x is not null) && doneIterating)`
`366`	`361`	`{`
`367`	`362`	`var array = _realm.Intrinsics.Array.ConstructFast(results);`
`368`		`- resolve.Call(Undefined, new JsValue[] { array });`
	`363`	`+ capability.Resolve.Call(Undefined, new JsValue[] { array });`
`369`	`364`	`}`
`370`	`365`	`}`
`371`	`366`
`@@ -392,8 +387,8 @@ void ResolveIfFinished()`
`392`	`387`	`}`
`393`	`388`	`catch (JavaScriptException e)`
`394`	`389`	`{`
`395`		`- reject.Call(Undefined, new[] { e.Error });`
`396`		`- return resultingPromise;`
	`390`	`+ capability.Reject.Call(Undefined, new[] { e.Error });`
	`391`	`+ return capability.PromiseInstance;`
`397`	`392`	`}`
`398`	`393`
`399`	`394`	`// note that null here is important`
`@@ -456,11 +451,11 @@ void ResolveIfFinished()`
`456`	`451`	`catch (JavaScriptException e)`
`457`	`452`	`{`
`458`	`453`	`iterator.Close(CompletionType.Throw);`
`459`		`- reject.Call(Undefined, new[] { e.Error });`
`460`		`- return resultingPromise;`
	`454`	`+ capability.Reject.Call(Undefined, new[] { e.Error });`
	`455`	`+ return capability.PromiseInstance;`
`461`	`456`	`}`
`462`	`457`
`463`		`- return resultingPromise;`
	`458`	`+ return capability.PromiseInstance;`
`464`	`459`	`}`
`465`	`460`
`466`	`461`	`// https://tc39.es/ecma262/#sec-promise.any`
`@@ -471,8 +466,6 @@ private JsValue Any(JsValue thisObject, JsValue[] arguments)`
`471`	`466`	`return capability.PromiseInstance;`
`472`	`467`	`}`
`473`	`468`
`474`		`- var (resultingPromise, resolve, reject, resolveObj, _) = capability;`
`475`		`-`
`476`	`469`	`var errors = new List<JsValue>();`
`477`	`470`	`var doneIterating = false;`
`478`	`471`
`@@ -487,7 +480,7 @@ void RejectIfAllRejected()`
`487`	`480`	`{`
`488`	`481`	`var array = _realm.Intrinsics.Array.ConstructFast(errors);`
`489`	`482`
`490`		`- reject.Call(Undefined, new JsValue[] { Construct(_realm.Intrinsics.AggregateError, new JsValue[] { array }) });`
	`483`	`+ capability.Reject.Call(Undefined, new JsValue[] { Construct(_realm.Intrinsics.AggregateError, new JsValue[] { array }) });`
`491`	`484`	`}`
`492`	`485`	`}`
`493`	`486`
`@@ -547,7 +540,7 @@ void RejectIfAllRejected()`
`547`	`540`	`return Undefined;`
`548`	`541`	`}, 1, PropertyFlag.Configurable);`
`549`	`542`
`550`		`- thenFunc.Call(item, new JsValue[] { resolveObj, onError });`
	`543`	`+ thenFunc.Call(item, new JsValue[] { capability.ResolveObj, onError });`
`551`	`544`	`}`
`552`	`545`	`else`
`553`	`546`	`{`
`@@ -560,11 +553,11 @@ void RejectIfAllRejected()`
`560`	`553`	`catch (JavaScriptException e)`
`561`	`554`	`{`
`562`	`555`	`iterator.Close(CompletionType.Throw);`
`563`		`- reject.Call(Undefined, new[] { e.Error });`
`564`		`- return resultingPromise;`
	`556`	`+ capability.Reject.Call(Undefined, new[] { e.Error });`
	`557`	`+ return capability.PromiseInstance;`
`565`	`558`	`}`
`566`	`559`
`567`		`- return resultingPromise;`
	`560`	`+ return capability.PromiseInstance;`
`568`	`561`	`}`
`569`	`562`
`570`	`563`	`// https://tc39.es/ecma262/#sec-promise.race`
`@@ -573,9 +566,6 @@ private JsValue Race(JsValue thisObject, JsValue[] arguments)`
`573`	`566`	`if (!TryGetPromiseCapabilityAndIterator(thisObject, arguments, "Promise.race", out var capability, out var promiseResolve, out var iterator))`
`574`	`567`	`return capability.PromiseInstance;`
`575`	`568`
`576`		`- var (resultingPromise, resolve, reject, _, rejectObj) = capability;`
`577`		`-`
`578`		`-`
`579`	`569`	`// 7. Let result be PerformPromiseRace(iteratorRecord, C, promiseCapability, promiseResolve).`
`580`	`570`	`// https://tc39.es/ecma262/#sec-performpromiserace`
`581`	`571`	`try`
`@@ -594,16 +584,16 @@ private JsValue Race(JsValue thisObject, JsValue[] arguments)`
`594`	`584`	`}`
`595`	`585`	`catch (JavaScriptException e)`
`596`	`586`	`{`
`597`		`- reject.Call(Undefined, new[] { e.Error });`
`598`		`- return resultingPromise;`
	`587`	`+ capability.Reject.Call(Undefined, new[] { e.Error });`
	`588`	`+ return capability.PromiseInstance;`
`599`	`589`	`}`
`600`	`590`
`601`	`591`	`// h. Let nextPromise be ? Call(promiseResolve, constructor, « nextValue »).`
`602`	`592`	`var nextPromise = promiseResolve.Call(thisObject, new JsValue[] { nextValue });`
`603`	`593`
`604`	`594`	`// i. Perform ? Invoke(nextPromise, "then", « resultCapability.[[Resolve]], resultCapability.[[Reject]] »).`
`605`	`595`
`606`		`- _engine.Invoke(nextPromise, "then", new[] { (JsValue) resolve, rejectObj });`
	`596`	`+ _engine.Invoke(nextPromise, "then", new[] { (JsValue) capability.Resolve, capability.RejectObj });`
`607`	`597`	`} while (true);`
`608`	`598`	`}`
`609`	`599`	`catch (JavaScriptException e)`
`@@ -612,13 +602,13 @@ private JsValue Race(JsValue thisObject, JsValue[] arguments)`
`612`	`602`	`// a. If iteratorRecord.[[Done]] is false, set result to IteratorClose(iteratorRecord, result).`
`613`	`603`	`// b. IfAbruptRejectPromise(result, promiseCapability).`
`614`	`604`	`iterator.Close(CompletionType.Throw);`
`615`		`- reject.Call(Undefined, new[] { e.Error });`
`616`		`- return resultingPromise;`
	`605`	`+ capability.Reject.Call(Undefined, new[] { e.Error });`
	`606`	`+ return capability.PromiseInstance;`
`617`	`607`	`}`
`618`	`608`
`619`	`609`	`// 9. Return Completion(result).`
`620`	`610`	`// Note that PerformPromiseRace returns a Promise instance in success case`
`621`		`- return resultingPromise;`
	`611`	`+ return capability.PromiseInstance;`
`622`	`612`	`}`
`623`	`613`
`624`	`614`
`@@ -715,6 +705,11 @@ JsValue Executor(JsValue thisObject, JsValue[] arguments)`
`715`	`705`	`ExceptionHelper.ThrowTypeError(engine.Realm, "reject is not a function");`
`716`	`706`	`}`
`717`	`707`
`718`		`- return new PromiseCapability(instance, resolve, reject, resolveArg, rejectArg);`
	`708`	`+ return new PromiseCapability(`
	`709`	`+ PromiseInstance: instance,`
	`710`	`+ Resolve: resolve,`
	`711`	`+ Reject: reject,`
	`712`	`+ RejectObj: rejectArg,`
	`713`	`+ ResolveObj: resolveArg);`
`719`	`714`	`}`
`720`	`715`	`}`