Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep track of the context in which an exception was thrown #95

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

andreubotella
Copy link
Member

This is needed for error events in the web integration (equivalent to Node.js's process.on("uncaughtException")).

The exact spec infrastructure for this is not yet settled, this is only one proposal.

This might be used by web specs like this:

  1. Let previousContext be AsyncContextSwap(someContext).
  2. Invoke someCallback. If this throws an exception err, then:
    1. Let throwContext be the surrounding agent's [[ThrowAsyncContextMapping]].
    2. If throwContext is EMPTY, set throwContext to AsyncContextSnapshot().
    3. Report the exception err with throwContext.

This is needed for `error` events in the web integration (equivalent
to Node.js's `process.on("uncaughtException")`).

This might be used by web specs like this:

>  1. Let `previousContext` be AsyncContextSwap(`someContext`).
>  2. Invoke `someCallback`. If this throws an exception `err`, then:
>     1. Let `throwContext` be the surrounding agent's
>        [[ThrowAsyncContextMapping]].
>     2. If `throwContext` is EMPTY, set `throwContext` to
>        AsyncContextSnapshot().
>     3. Report the exception `err` with `throwContext`.
@andreubotella
Copy link
Member Author

After some thought, I'm not sure that having the error event have the context in which the exception was thrown (as opposed to the context that caused that task to be run) would be useful.

asyncVar.run("foo", () => {
  setTimeout(() => {
    asyncVar.run("bar", () => {
      throw new Error();
    });
  }, 1000);
});

window.onerror = () => {
  console.log(asyncVar.get());
};

In the above code, the current spec with the proposed web integration would log "foo", but with this PR (and the proposed HTML spec changes) it would log "bar".

The reason I'm not sure this would be useful is that we see two main use cases for this proposal: tracing, and tracking provenance from concurrent isolated applications (see #107 for more on this distinction). For tracing, according to @legendecas there's not a clear benefit for this PR, and in the case of tracking isolated applications, you would rarely have a case where this would make a difference.

However, one case where there would be a difference for isolated applications is if an error happens during or synchronously after the initialization of such an application:

// The context here is the empty context.

for (let i = 0; i < 10; i++) {
  setTimeout(() => {
    asyncVar.set(i, async () => {

      if (Math.random() > 0.5) {
        // Throwing here would fire an error event with the empty context.
        throw new Error();
      }

      await doSomething();

      if (Math.random() > 0.5) {
        // Throwing here would fire an unhandledrejection event with the correct context.
        throw new Error();
      }

    });
  }, 0);
}

How relevant is this for that use case?

@legendecas
Copy link
Member

However, one case where there would be a difference for isolated applications is if an error happens during or synchronously after the initialization of such an application:

// The context here is the empty context.

for (let i = 0; i < 10; i++) {
  setTimeout(() => {
    asyncVar.set(i, async () => {

      if (Math.random() > 0.5) {
        // Throwing here would fire an error event with the empty context.
        throw new Error();
      }

      await doSomething();

      if (Math.random() > 0.5) {
        // Throwing here would fire an unhandledrejection event with the correct context.
        throw new Error();
      }

    });
  }, 0);
}

How relevant is this for that use case?

In this example, the anonymous function passed to asyncVar.set is an async function, the first throw should result in a rejection of the returned promise. It should not fire an error event. I'm not sure I understand this example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants