Skip to content

Commit 79f3a7e

Browse files
mstoykovdop251
andauthored
Promise resolve and reject now return Interrupt errors (#624)
* Promise resolve and reject now return Interrupt errors This is so that it can be handled at all. Previously the error was just not propagated which lead to interrupts not really working with asynchronous code. Fixes #623 * Apply suggestions from code review Co-authored-by: Dmitry Panov <[email protected]> * fixup! Promise resolve and reject now return Interrupt errors --------- Co-authored-by: Dmitry Panov <[email protected]>
1 parent 5f46f27 commit 79f3a7e

File tree

2 files changed

+46
-5
lines changed

2 files changed

+46
-5
lines changed

builtin_promise.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -595,14 +595,17 @@ func (r *Runtime) getPromise() *Object {
595595
return ret
596596
}
597597

598-
func (r *Runtime) wrapPromiseReaction(fObj *Object) func(interface{}) {
598+
func (r *Runtime) wrapPromiseReaction(fObj *Object) func(interface{}) error {
599599
f, _ := AssertFunction(fObj)
600-
return func(x interface{}) {
601-
_, _ = f(nil, r.ToValue(x))
600+
return func(x interface{}) error {
601+
_, err := f(nil, r.ToValue(x))
602+
return err
602603
}
603604
}
604605

605606
// NewPromise creates and returns a Promise and resolving functions for it.
607+
// The returned errors will be uncatchable errors, such as InterruptedError or StackOverflowError, which should be propagated upwards.
608+
// Exceptions are handled through [PromiseRejectionTracker].
606609
//
607610
// WARNING: The returned values are not goroutine-safe and must not be called in parallel with VM running.
608611
// In order to make use of this method you need an event loop such as the one in goja_nodejs (https://github.com/dop251/goja_nodejs)
@@ -617,11 +620,12 @@ func (r *Runtime) wrapPromiseReaction(fObj *Object) func(interface{}) {
617620
// go func() {
618621
// time.Sleep(500 * time.Millisecond) // or perform any other blocking operation
619622
// loop.RunOnLoop(func(*goja.Runtime) { // resolve() must be called on the loop, cannot call it here
620-
// resolve(result)
623+
// err := resolve(result)
624+
// // Handle uncatchable errors (e.g. by stopping the loop, panicking or setting a flag)
621625
// })
622626
// }()
623627
// }
624-
func (r *Runtime) NewPromise() (promise *Promise, resolve func(result interface{}), reject func(reason interface{})) {
628+
func (r *Runtime) NewPromise() (promise *Promise, resolve, reject func(reason interface{}) error) {
625629
p := r.newPromise(r.getPromisePrototype())
626630
resolveF, rejectF := p.createResolvingFunctions()
627631
return p, r.wrapPromiseReaction(resolveF), r.wrapPromiseReaction(rejectF)

runtime_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1792,6 +1792,43 @@ func TestInterruptInWrappedFunctionExpectStackOverflowError(t *testing.T) {
17921792
}
17931793
}
17941794

1795+
func TestInterruptWithPromises(t *testing.T) {
1796+
rt := New()
1797+
rt.SetMaxCallStackSize(5)
1798+
// this test panics as otherwise goja will recover and possibly loop
1799+
rt.Set("abort", rt.ToValue(func() {
1800+
// panic("waty")
1801+
rt.Interrupt("abort this")
1802+
}))
1803+
var queue = make(chan func() error, 10)
1804+
rt.Set("myPromise", func() Value {
1805+
p, resolve, _ := rt.NewPromise()
1806+
queue <- func() error {
1807+
return resolve("some value")
1808+
}
1809+
1810+
return rt.ToValue(p)
1811+
})
1812+
1813+
_, err := rt.RunString(`
1814+
let p = myPromise()
1815+
p.then(() => { abort() });
1816+
`)
1817+
if err != nil {
1818+
t.Fatal("expected noerror but got error")
1819+
}
1820+
f := <-queue
1821+
err = f()
1822+
if err == nil {
1823+
t.Fatal("expected error but got no error")
1824+
}
1825+
t.Log(err)
1826+
var soErr *InterruptedError
1827+
if !errors.As(err, &soErr) {
1828+
t.Fatalf("Wrong error type: %T", err)
1829+
}
1830+
}
1831+
17951832
func TestRunLoopPreempt(t *testing.T) {
17961833
vm := New()
17971834
v, err := vm.RunString("(function() {for (;;) {}})")

0 commit comments

Comments
 (0)