Promise API feedback

[mstoykov]

Promise ... identification, equalness, hashability issue

I have been working on adding an event loop to k6 and a this definitely shows that HostPromiseRejectionHandler usability is ... a problem.

I tried to copy the deno behaviour of "if a rejection doesn't get handled until the next time there are no task in the microtask queue - abort". Which seems fine but requiers something like

func addPromiseRejectionTracker(rt *goja.Runtime) {
	// this is super innefficient but no other way to keep track :( so I guess it needs to do for now
	inFlightPromises := make([]*goja.Promise, 10)

	rt.SetPromiseRejectionTracker(func(p *goja.Promise, op goja.PromiseRejectionOperation) {
		// Read Notes on https://tc39.es/ecma262/#sec-host-promise-rejection-tracker
		if op == goja.PromiseRejectionReject {
			inFlightPromises = append(inFlightPromises, p)
			holdingPromise, resolve, _ := rt.NewPromise()
			call, _ := goja.AssertFunction(rt.ToValue(holdingPromise).ToObject(rt).Get("then"))
			call(rt.ToValue(holdingPromise), rt.ToValue(func() {
				for _, promise := range inFlightPromises {
					if promise == p {
						common.Throw(rt,
							fmt.Errorf("Uncaught (in promise) %s", p.Result()))
						return
					}
				}
			}))
			resolve(nil)
		} else { // goja.PromiseRejectionHandle so a promise that was previously rejected without handler now got one
			for i, promise := range inFlightPromises {
				if promise == p {
					// remove this promise
					inFlightPromises = append(inFlightPromises[:i], inFlightPromises[i+1:]...)
					return
				}
			}
		}
	})
}

(This codes ... in practice for now adds another promise instead of being directly putted on the event loop, which is as I want it to be added before the event loop but the problem I will discuss stays the same)

As you can see I have to some how remember that I have done something for a promise (add it to the queue of rejected ones that I should abort on) and then on a (potential) later call find that promise and abort the action. But as a promise is not hashable I in practice do linear search in a slice. This likely isn't that big of a problem as I expect this will be rare occurance and I will be aborting after this so ... 🤷.

I do wonder though if we instead we can have some workaround for this, the two things coming to mind:

1. simple and stupid - have a unique hashable identification for a promise, it can literally be a int64 that goes up every time a promise is made. I really do hope we won't get to the upper limit of that ;)
2. probably better, but also more involved - have a way to add HostPromiseRejectionHandler per promise. This will make it possible to in this case just set it to something that unqueues it through w/e mechanism seems most useful for the developer:

func addPromiseRejectionTracker(rt *goja.Runtime) {
	// this is super innefficient but no other way to keep track :( so I guess it needs to do for now
	inFlightPromises := make([]*goja.Promise, 10)

	rt.SetPromiseRejectionTracker(func(p *goja.Promise, op goja.PromiseRejectionOperation) {
		// Read Notes on https://tc39.es/ecma262/#sec-host-promise-rejection-tracker
		if op == goja.PromiseRejectionReject {
			inFlightPromises = append(inFlightPromises, p)
			holdingPromise, resolve, _ := rt.NewPromise()
                        var handled bool
			call, _ := goja.AssertFunction(rt.ToValue(holdingPromise).ToObject(rt).Get("then"))
			call(rt.ToValue(holdingPromise), rt.ToValue(func() {
				if !handled {
					common.Throw(rt,
						fmt.Errorf("Uncaught (in promise) %s", p.Result()))
					return
				}
			}))
                        p.SetPromiseRejectionTracker(func() {handled=true}
			resolve(nil)
		}
	})
}

(code written in this, so some small msitakes might exist ;) , but I hope you get the idea).

HostEnqueuePromiseJob

After this change which landed in ES2020 there is now HostEnqueuePromiseJob while previously to that change (at least to me) it sounded like goja needs to implement the Promise queue this addition makes it seem otherwise. I am probably wrong as at least in test262 there are zero references to that so it seems like it's still expected tobe implemented in the engine 🤷 .

I was just wondering what your opinion on implementing this is, at least for now I don't have a use for it, but it will still be nice to have and as I am working on an event loop (mostly it's integration) I am wondering if I need to get ready to queue promises on my own :).

And in conclusion - thank you a lot for all the work 🎉 . Hopefully after I finish this I can go back to getting import/export support in goja 🤞

[dop251]

A pointer to a Promise can be used as a hashable identifier, i.e. you can have map[*Promise]whatever.

[mstoykov]

A pointer to a Promise can be used as a hashable identifier,

🤦 I was absolutely certain that pointers aren't a valid identifier and even opened the specification at some point in researching this ... apperantly I didn't re-read it though 🤦

[mstoykov]

What is your opinion on HostEnqueuePromiseJob ?

[dop251]

I'm a bit confused. HostEnqueuePromiseJob is defined as a hook, but it is supposed to do the actual queueing, i.e. it doesn't sound to me like serves the same purpose as PromiseRejectionTracker (which is a hook that may have custom implementation). I guess from ECMAScript point of view they are the same, but from goja point of view they are not, I can't make it a hook (like PromiseRejectionTracker) because it won't have the necessary access to the Runtime to do the actual queueing.

What do you need if for?

[mstoykov]

Sorry for the long delay - I started answering and went to look something up ... and well here we are :(.

I don't have a particular use case really. I found while investigating and reading the spec implementing the event loop in k6.

In our case we have stranger (than usual for event loops) requirement - we actually need to exit the event loop (and end the iteration) if nothing is happening in the background and only then. Otherwise code such as

export default () => {
  makeHTTPRequestAndReturnPromise().then((resp) => {/* do something */})
}

will just make promises and loop. (Note: for this use case k6 just runs the default function forever as fast as possible)
But in reality (at least in my experience asking people) everyone expects that the iteration will end only once that promise is resolved.

If HostEnqueuePromiseJob existed ... I doubt that would've made it any easier though. (The hard parts weren't actually around this to begin with so :shrug).

A thing that would've helped is to be notified when a promise was created and resolved/rejected. The second part more or less being what HostEnqueuePromiseJob and PromiseRejectionTracker are doing. This would've meant that at least goja.NewPromise would've been more "cleanly" used inside k6 code. Even now this is an extremely small problem, definitely not worthy of the amount API needed IMO. I definitely will not rewrite the event loop to support this in the first version ;).

Additionally a particular case that we currently have no way of handling is

var res, rej, iter = 0;
export default function() {
  if (iter ==0){ // do this only the first time
    new Promise((r, j)=> {
      res = r;
      rej = j;
    }).then((s) => console.log(__ITER, s))
  } else {
    res(5);
  }
  iter++;
}

In this particular example in one iteration of k6 a user will create a promise and then on the next iteration resolve it. If we are really strict this should not be possible. But in reality global state is already shared between iterations so this isn't really a problem for us. Additionally unlike the previous example this is unlikely code, hopefully 🤞 . And the only way for this to be "fixable" in k6 is for goja to have a way to signal any change to a Promise, including it's creation, which seems like a lot of hooks/triggers.

So in retrospect I do think that HostEnqeuePromiseJob implementation in goja isn't really worth the hastle (for the k6 project) and the question was mostly about what your opinion on it is.

p.s:

I can't make it a hook (like PromiseRejectionTracker) because it won't have the necessary access to the Runtime to do the actual queueing.

I would expect that if I use HostEnqueuePromiseJob, goja will not have any internal queue, I will be the one who will have to queue and execute them. If that is what you meant 🤔

I am leaving this open in case you want to discuss something more, but you can close it otherwise. cliecked the wrong button 🤦 , leaving it as is, I guess you can open it if you want to discuss more 😅

Thanks again for all the work 🙇