The right context for error/rejection handlers

[andreubotella]

The web, as well as some other host environments, have error handlers for uncaught exceptions in synchronous code (in the web, the error event), and for unhandled promise rejections (unhandledrejection in the web). For certain use cases, such as aborting async operations associated with an async control flow when something fails in it, it's important for the error handlers to have access to the context of that async control flow. However, propagating the right context, let alone deciding which is right, is somewhat tricky.

The error event

In the web, every time that this event is fired (with the single exception of the window.reportError() API), it is fired immediately after some piece of spec text / browser code has called into userland JS code that threw an exception. If userland JS code is called when the JS stack is empty (i.e. the JS code is called asynchronously, or in a task), this always happens. If the stack is not empty (i.e. the JS code is called synchronously from some API), sometimes the error is rethrown, but other times the error event is fired (i.e. in event listeners).

So consider the following code:

function cb() {
  asyncVar.run("bar", () => {
    throw new Error();
  });
}

asyncVar.run("foo", () => {
  setTimeout(() => cb, 0);
});

The moment that the error is thrown, the active context has asyncVar set to "bar", but when the exception propagates outside of the .run in cb, that context gets lost. By the time that the exception reaches the browser code that called cb, the only possible context that could be active is "foo", since it's the one that setTimeout propagated. Is this a behavior we want?

Note that lately we're leaning towards not propagating the context in most events which are fired asynchronously, letting userland code handle that. For those events, even if the event listener set the right context itself, or was wrapped with AsyncContext.Snapshot.wrap, the error event would still be associated with the empty context.

There is a way to work around this in userland, by catching the exception before it propagates upwards to browser code, and calling window.reportError() on the right context. But this is boilerplate that would be really easy to miss.

For what's worth, I had previously open an issue and PR (#90, #95) to store the context in which an exception is thrown, which would correctly give the result "bar" in this case. However, this involved patching the behavior of try, which we probably shouldn't do willy-nilly, and this would also give the wrong result for catch blocks that rethrow the exception. See below for what might be a better alternative to this.

The unhandledrejection event

This event is fired whenever a promise is rejected without being handled. Since awaiting a promise handles it, this event can only be fired when a promise is created and not awaited (including things like calling an async function without awaiting it). This roughly maps to the start of a parallel async execution, so it can be seen as parallel to the behavior of web APIs like setTimeout or queueMicrotask with the error event.

However, since userland JS code can start such a parallel async execution by simply creating a rejected promise without awaiting it, there's no web API that can handle this automatically. Instead, the JS spec has a host hook that tells the HTML spec that a promise was rejected without handlers. In the spec, this hook is called synchronously when the promise is rejected, so it has the rejection's context (though again, see below), but the HTML spec delays the firing of the event, so we'd have to propagate the context in the HTML spec if we want to expose it.

However, as for the error event, this context might not be the one you'd want – in this case because rejections can be propagated across promises.

Let's look at a promise chain:

const p = Promise.withResolvers();

asyncVar.run("foo", () => {
  p.promise.then(value => console.log(value));
});

asyncVar.run("bar", () => {
  p.reject(new Error());
});

p.promise rejects with context "bar", as expected. However, the call to .then handled that promise and returned a new one, which is the one that rejects without handlers. Since that call to .then has the "foo" context, the handler callbacks, and the rejection of this second promise, happen in that context. Since the rejection of this second promise happens in a microtask after p.promise was rejected, the "bar" context is not preserved, and forcefully preserving it just for this case would add complexity.

So for promise chains, we can only rely on the rejection context if the rejection originated on the last promise in the chain. What about for async callbacks?

async function test() {
  await doSomething();
  await asyncVar.run("bar", async () => {
    await doSomething();
    throw new Error();
    await doSomething();
  });
  await doSomething();
}

asyncVar.run("foo", () => {
  test();
});

awaits basically act like .then that run in the context of the containing async function, with code between awaits roughly being placed in the fulfillment callback of .then. For the asyncVar.run callback, this means that the promise it returns has the rejection context "bar", since the callback is called with that context, and therefore the awaits behave like .thens in that context.

But this promise is handled by the await in the test function, so its rejection context gets lost. Instead, since test runs in the "foo" context, and its awaits behave like .then called in that context, the rejection context that the test promise ends up having is "foo".

In the end async functions with unhandledrejection end up behaving just like sync functions with error.

There is an async counterpart to the sync solution in #90 / #95, which is to propagate the rejection context of a promise to the following promise in the chain if the reaction did not have a rejection handler (i.e. if p rejects with a context, p.then(someFn) would reject with the same context). This also has the issue that rethrowing a rejection (or doing .catch(err => Promise.reject(err))) loses the context. We also have not prototyped it or looked into it yet, and there might be other issues due to the different ways promise resolutions can propagate.

Alternative: store the context in the Error

The context in which an exception is thrown, or a promise is rejected, is basically control flow information related to the exception throwing or promise rejection. And there is already some control flow information related to exception throwing that is currently exposed in JS, in the (non-standard but basically universal) stack property of Error objects.

So an alternative proposal is to not expose any context on the error and unhandledrejection events, and instead store the current context whenever an Error instance is created, exposing it as a property of type AsyncContext.Snapshot. This is not necessarily the same as the context where the exception is thrown, but the stack property is also set on construction and might also be different from where the error object is thrown. In practice, this difference is unlikely to matter.

One important issue with this approach is that every Error instance would keep alive the context in which it was created, along with all of the values set for each AsyncContext.Variable in that context. This would likely increase the memory footprint of code that might have been written for a long time; and even for new code, the context storage is implicit and the developer often would not be aware of how much memory it's referencing. So this might be a deal-breaker. But it seems like otherwise it's better than the other options.