Detect unhandled promise rejections

[augustoroman]

See https://github.com/discourse/mini_racer/issues/82

This library suffers the same problem.

[jeromegn]

Yes, that would be very useful.

Would it not just be a matter of calling a function with a callback into go?

[augustoroman]

Yeah, that's one solution, which may be the right thing. I was originally thinking that it would be caught in Eval and the error value of Eval should return an UnhandledPromiseRejectionError, but I think that maybe just registering an Isolate-scoped or Context-scoped callback so that the user of the v8 library can handle it themselves directly is the right solution.

[augustoroman]

Soooo, I'm looking into this and trying to figure out what the right way of handling this is. We have the same problem that Context callbacks have here:

• When we get a callback in C++, we have to call back into Go and tell the Go code which Isolate the notification occurred in.
• We can't pass Go's Isolate pointers to C++.
• If we store a map of (some handle) to *Isolate in Go, then we'll screw up Go's GC of Isolates.

I think we can make the assumption that V8 will only ever call the unhandled promise callback while we are calling into c from go. If that's the case, then we can use the existing Context id -> Context map... I'm just worried that I'm missing some case where the callback will be called unexpectedly. I guess we could at least log the offending call on the Go side if we can't locate the appropriate Context.

[jeromegn]

Unhandled promise rejections can happen at random moments, I don't think assuming they might only happen when calling C from go is right. I bet it'll often happen right after we're done.

If we manually called RunMicroTasks, then this would be more deterministic. Not sure we want to do that though.

How do you mean we can't pass go pointers to C? Would they get GCed too fast? Passing go pointers back and forth would probably work.

I had started on V8 bindings with the Crystal language and used some kind of dispatcher pattern using channels. Maybe we just store map[string]chan *v8.Value which would be a map of "id" -> callback chan. Each time you "listen" for the unhandled promise rejection callback, you create a new chan and a unique id you set up in that map (probably sync.Map, come to think of it) and naively dispatch messages into the proper channel.

My Crystal implementation used a similar pattern, but for locking an isolate (like ISOLATE_SCOPE) and creating a context scope (like VALUE_SCOPE). You could then run a bunch of operations from Crystal without creating a new scope for every call into C. Probably only makes sense because of Crystal's zero-cost abstraction C bindings. I might do some benchmarks.

[augustoroman]

We can't pass go pointers to C because of the cgo rules. See the comments in v8.go about callbacks.

I don't think they can happen at any moment -- V8 doesn't have any concept for sleep or time-based operations on it's own. I think the RunMicroTasks is exactly when it can happen, and I suspect that's run automatically by default.

[jeromegn]

We can't pass go pointers to C because of the cgo rules. See the comments in v8.go about callbacks.

Ah, good to know!

I think the RunMicroTasks is exactly when it can happen, and I suspect that's run automatically by default.

Yes, pretty sure those are ran automatically by default. My point is, we don't know when that's going to run and so we don't know when the callback will be called.

Any thoughts on dispatching callbacks via channels?

[augustoroman]

Channels won't solve the problem, unfortunately, because we'll have to construct the appropriate *v8.Value objects which internally have pointers to the Context (and, therefore the Isolate). So we don't lose the requirement of having to track the pointers somewhere.

[jeromegn]

I think you can get most of that information from C++ without doing much in go.

static void OnPromiseReject(PromiseRejectMessage message) {
    Local<Promise> promise = message.GetPromise();

    // Get the isolate
    Isolate* isolate = promise->GetIsolate();

    v8::PromiseRejectEvent event = message.GetEvent();

    // Get the current context
    Local<Context> context = isolate->GetCurrentContext();

    Local<Value> value = message.GetValue();
    if (value.IsEmpty())
      value = v8::Undefined(isolate);

    _go_promise_reject_callback(/* context, value, etc. */);
}

^ not entirely my code, copy pasted from somewhere so there are probably a lot wrong with it!