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

WebAssembly.Exception is not a subclass of JS Error #183

Closed
kmiller68 opened this issue Sep 22, 2021 · 46 comments
Closed

WebAssembly.Exception is not a subclass of JS Error #183

kmiller68 opened this issue Sep 22, 2021 · 46 comments

Comments

@kmiller68
Copy link

Since WebAssembly.Exception is a JS subclass of JS's Error class it seems like it should be possible to provide the message and cause properties from the constructor. Currently, the WebAssembly.Exception constructor only takes the WebAssembly.Tag and the tag's parameters. Is the inability to provide a message or cause intentional or an oversight?

@eqrion
Copy link
Contributor

eqrion commented Sep 22, 2021

I was under the understanding that WebAssembly.Exception was not a subclass of JS's Error class.

@kmiller68
Copy link
Author

@eqrion Ah, yeah you're correct. I guess then this should be retitled to should it be a subclass of JS Error? I'd expect developers would want to know the stack from which their exception originated when they get an uncaught exception.

@kmiller68 kmiller68 changed the title WebAssembly.Exception constructor has no way to pass a message or cause WebAssembly.Exception is not a subclass of JS Error Sep 22, 2021
@eqrion
Copy link
Contributor

eqrion commented Sep 22, 2021

I believe this was intentional and discussed in some other threads, but it's hard for me to find definitive statements on it. This comment seems relevant. If that's the case and still valid, it may be good to formally state that somewhere.

@kmiller68
Copy link
Author

Yeah, I can see the argument for not gathering stack traces eagerly. On the other hand, I'd guess there's a significant number of developers that want those stack traces for diagnostic reasons. Perhaps we should have a way to natively create an exception with a stack trace? I suppose an alternative would be to pass a JS Error object as one of the tag's parameters but that still requires calling to JS.

@thibaudmichaud
Copy link
Collaborator

thibaudmichaud commented Sep 27, 2021

Note that in V8 there is already a way to opt-out of stack trace capture by setting the "Error.stackTraceLimit" property to 0: https://v8.dev/docs/stack-trace-api. This is not standardized though, but if other engines provide a similar functionality I think that's a very strong argument in favor of making wasm exceptions inherit from JS Error. This provides an easy and efficient way to enable/disable stack trace collection for wasm exceptions.

@fgmccabe
Copy link

There is a fundamental 'problem' with automatically collecting stack traces for wasm exceptions: the information may be useful in the case of diagnosing an unhandled exception; but many languages have very specific requirements for what stack trace information to collect and these cannot currently be handled in the EH proposal without considerable heavy lifting. (E.g., Java's stack trace is for the entire stack at the point of creating the exception (in terms of methods called), Python lets you control how much of a trace to generate, C# normally records a stack trace when the exception is thrown)
The second main issue is that collecting traces is inherently expensive and doing so would have a chilling effect on using exceptions for control flow purposes.

@aheejin aheejin mentioned this issue Sep 27, 2021
11 tasks
@aheejin
Copy link
Member

aheejin commented Sep 27, 2021

@fgmccabe

There is a fundamental 'problem' with automatically collecting stack traces for wasm exceptions: the information may be useful in the case of diagnosing an unhandled exception; but many languages have very specific requirements for what stack trace information to collect and these cannot currently be handled in the EH proposal without considerable heavy lifting. (E.g., Java's stack trace is for the entire stack at the point of creating the exception (in terms of methods called), Python lets you control how much of a trace to generate, C# normally records a stack trace when the exception is thrown)
The second main issue is that collecting traces is inherently expensive and doing so would have a chilling effect on using exceptions for control flow purposes.

One thing I'd like to make clear is that the current spec doesn't mandate stack traces generation now. So the current problem is more about figuring out how we are going to get stack traces and not how we make the engine faster in face of generating the stack traces.

Also I don't think we can standardize the stack trace format from our JS API. By the way I just found out even Error.stack itself is not a standardized thing... V8 and FF implement it but apparently it's not a standardized property.

@aheejin
Copy link
Member

aheejin commented Sep 27, 2021

While I don't think we should mandate stack trace generation, there are several levels to it. Higher number means less mandatory.

  1. We mandate stack trace generation for all engines. I don't think anyone is arguing for this.

  2. We mandate stack trace generation in JS API, making WebAssembly.Exception inherit from JS Error. This will only affect web VMs.

3-a. We make WebAssembly.Exception inherit from JS Error, but as @thibaudmichaud pointed out, that may not mean we should generate stack traces. So stack trace generation is optional. (But as he said if it's not a standard feature, can we make our spec rely on that?)
3-b. We make WebAssembly.Exception take a boolean saying whether to generate stack traces or not. Depending on this parameter, the web VM can decide whether to use JS Error or not.
3-c. We don't say anything about JS Error in JS API, but each toolchain can devise its own way to generate stacktraces. For example, instead of a toolchain generating wasm throw instruction, create a JS function that throws JS Error and import it from wasm and use it. (Not sure if it's ideal, but listing it here just to be exhaustive.)

(The formatting here seems messed up, sorry, not sure how to fix)

Searching the repo for previous discussions, I found #150 (comment). @takikawa @rossberg Any opinions on this?
Also cc @Ms2ger who worked much on JS API.

@dschuff
Copy link
Member

dschuff commented Sep 27, 2021

At a high level I really would like to be able to support both the use case of getting diagnostic info from uncaught (or not-immediately-caught) exceptions (which as @kmiller68 mentioned, includes messages as well as stack traces) and also the use case of using EH for control flow which would be harmed by taking stack traces on every throw. IMO that means there should be some way 1) to throw from wasm without any stack trace or message generation, and also 2) to have an Error-shaped object propagate out from C++-style exceptions. Ideally the content of that object matches what web users would expect; i.e. it would be the same as what web engines already generate by default from JS exceptions (even if that content isn't fully specified). Note also that the latter use case is scoped to JS.

I think that for Emscripten it would actually be OK to to call out to JS code to throw C++ exceptions (i.e. instead of using the wasm throw instruction directly). Making it throw JS Error instead of WebAssembly.Exception (which I think is the intent of 3-c) would be problematic however, because then C++ exceptions would be indistinguishable from JS exceptions originating from user JS code (currently emscripten handles C++ exceptions directly but just runs destructors when JS or other foreign exceptions propagate).
Adding arguments to the WebAssembly.Exception constructor to allow it to optionally take a message and generate a stack trace seems OK to me in principle. It would mean that these objects might or might not have the fields that would be expected of a JS Error, and any user code that deals with unhandled exceptions would have to be prepared for that. This can happen in JS already (when things other than Errors are thrown), so maybe that's ok. But I don't really know enough about JS API design to know whether that's a good idea.
This also means that in this case there could be an asymmetry between the wasm throw instruction and the JS API. I think I'd prefer to leave the throw instruction as "bare" (it would take no arguments other than the thrown value, and it would not generate messages or traces when used on the web), and make JS users explicitly allocate a WebAssembly.Exception if they want a stack trace. The other option would be to have some kind of way to make a core-wasm throw instruction also generate a message and/or stacktrace, but 1) we wouldn't really have any way in the core spec to say what exactly that info would look like, and 2) there would still be an asymmetry, since on the wasm side the stack trace would be taken on throw and on the JS side it would still be presumably taken on exception object allocation, as it is in JS today.

So all that is to say, tl:dr I like something like option 3-b, with a non-tracing throw instruction.

@aheejin
Copy link
Member

aheejin commented Oct 19, 2021

I generally agree with what @dschuff said. I think whether WebAssembly.Exception should be a JS Error can be an implementation detail and doesn't have to be specified in the spec, which also means I don't think we need to either mandate or ban it. I also think, as I said in the previous comment, stack traces should not be mandatory.

I propose we choose one of these two options:

  1. We make WebAssembly.Exception take a boolean saying whether to generate stack traces or not. Depending on this parameter, the web VM can decide whether to use JS Error or not. Because a bare wasm throw instruction cannot access this option, I think toolchains should provide a way to access this option, i.e., providing a host function that throws WebAssembly.Exception with the stack trace option on.
  2. We don't say anything about JS Error in JS API, but each toolchain can devise its own way to generate stack traces. For example, instead of a toolchain generating wasm throw instruction, create a JS function that throws WebAssembly.Exception in JS. The toolchain can insert additional properties to this Exception object to this object before throwing, such as message or stack (stack traces). Also the toolchain can even make its prototype point to JS Error so that it will behave like JS Error. The point is these are implementation details, and we trust toolchains to do this rather than specifying anything in the JS API.

(These are basically some adaptation of 3-b and 3-c in my previous comment.)

Both options would probably need that toolchains provide an option for stack traces, and if it's on, wasm code uses an imported host function (which throws WebAssembly.Exception) when throwing an exception. When it's off, wasm code uses a bare wasm throw. This option can be on or off by default depending on the language and VM. If the language requires fast exceptions this should be off; if not it can be on.

I'd appreciate any opinions on the preference of either of these options. Also if you have another alternative not listed here I'd appreciate that too.

@Ms2ger
Copy link
Contributor

Ms2ger commented Oct 19, 2021

I don't have a strong opinion either way, but I strongly disagree with this:

I think whether WebAssembly.Exception should be a JS Error can be an implementation detail and doesn't have to be specified in the spec

Authors should be able to rely on the same code working across browsers, whether that's with or without subclassing or stack traces.

@RossTate
Copy link
Contributor

Both options would probably need that toolchains provide an option for stack traces, and if it's on, wasm code uses an imported host function (which throws WebAssembly.Exception) when throwing an exception. When it's off, wasm code uses a bare wasm throw.

When the issue of stack traces was discussed long ago, I was told that a key requirement was that tooling not have to differ depending on whether stack traces were enabled or disabled, even if that difference was just a change to what import gets provided to the module (rather than the much more extensive difference suggested above).

If that requirement is no longer in place, then we can consider making stack traces an explicit part of the payload. In the JS API, one can indicate that a specific externref-typed index in the tag's payload provides the stack trace. In the wasm module, one can import a $__get_stack_trace : [] -> [externref] function (which can simply return null when the person instantiating the module wants stack traces to be disabled) and use that to generate the stack trace to put in the payload of the throw instruction.

In addition to giving applications a lot more control over a feature that can have a 6x impact on performance, making stack traces explicit enables us to greatly simplify the proposal. At present, the difference between rethrowing an exception and throwing an exception with the same tag and payload is just the stack trace. So by making this implicit difference explicit, we can eliminate the need for the most complicating aspects of this proposal: rethrow/delegate/catch_all (leaving us with just try/catch/throw).

Those simplifications also make addressing #158 and #185 easy as there are no longer instructions whose placement is severely restricted.

The impact of this change would be minimal on engines because, besides removing instructions, it only affects the JS API. There would be more substantial changes required of tooling, but it sounds such changes likely need to happen anyways.

@aheejin
Copy link
Member

aheejin commented Oct 19, 2021

@RossTate

  1. This issue is discussing a very specific aspect of JS API based on the current core spec, so if you are proposing drastic changes to the current core spec, please do it in a new issue. I will not discuss core spec changes in this issue anymore after this comment.
  2. We are aware that you wanted to remove many instructions before, but I think we discussed them for a long time last year already, and decided to settle on the current spec. I'd like to refrain from relitigating all those again given that we are planning to advance to Phase 4 in near future. Also some of the arguments you mentioned, including delegate or Encoding Exceptional CFGs #185, are not even related to stack traces at all. But again, let's not do that in this issue.

@RossTate
Copy link
Contributor

The following was a design suggestion for just the JS API:

In the JS API, one can indicate that a specific externref-typed index in the tag's payload provides the stack trace. In the wasm module, one can import a $__get_stack_trace : [] -> [externref] function (which can simply return null when the person instantiating the module wants stack traces to be disabled) and use that to generate the stack trace to put in the payload of the throw instruction.

@rossberg
Copy link
Member

I think whether WebAssembly.Exception should be a JS Error can be an implementation detail and doesn't have to be specified in the spec

It is observable in JS (e.g. through their prototypes) whether two classes are identical, in a subclass relation, or unrelated. So I agree with @Ms2ger that we cannot get away with not specifying it one way or the other.

@dschuff
Copy link
Member

dschuff commented Oct 20, 2021

Regarding the subclass relationship: I agree that we should specify the browser behavior to the extent possible (modulo existing issues such as the format of the stack trace text). One thing we could do is say that the thrown objects have e.g. a stack property (and perhaps message and/or cause properties) but allow it to be empty or have some specified non-stack string. AFAICS the contents of these properties (especially stack) are not really specified anyway. This approach would allow us to specify that WA.Exception is a subclass of Error (or alternatively, even say that it's not a subclass, but give it the properties that generic handler code would be expecting... I'm not sure if that would be useful or not). In that case it would make sense to add come constructor arguments as @kmiller68 suggested. Taken together, that would allow exceptions created by wasm throw instructions and those created with the JS API to be as alike as possible, and also allow richer exception objects for systems that are willing to call out to JS to create them.

[edit: I originally considered an idea where the VM could generate a call to user JS code to collect the trace or not, but all of those ideas have downsides, so I deleted that part].

So given that I think I still prefer a version of the above paragraph; specifically do the following 2 things:

  1. Keep the status quo for bare wasm throw, but make WebAssembly.Exception a subclass of Error, and let the metadata fields (which are not standardized) be empty when generated from throw. This would allow the object to be created lazily (if an exception never propagates into JS, then it would not need to be created at all).
  2. Also add some arguments to the WebAssembly.Exception constructor to create an object with the right metadata, so that a toolchain which wants to opt into full metadata can create one in JS and throw it.

@RossTate
Copy link
Contributor

I'm not sure my suggestion was understood. There was no automatic collection of stack traces or special imports.

But I thought of a refinement of that suggestion which happens to line up with @dschuff's lazy semantics, so hopefully that's an indication that we're converging. When you define a new exception tag, in addition to specifying its type [t*], also specify a function id $func of a function of type [t*] -> [externref]. For the JS API, when a throw $exn reaches a JavaScript catch (e) (or the top of the stack, i.e. is uncaught), this function gets called to convert the payload of the wasm exception into the JS value to be caught.

This provides a lot of flexibility. For example, a module could implement this conversion function by calling an imported function that constructs some subclass of Error, generating the stack trace from that point. Or a module could choose to use an imported function to generate a stack trace at the point the exception was first constructed internally, and then implement this conversion function by constructing some subclass of Error with this preexisting stack trace (which it could retrieve from the payload, or some global, or some table depending on how it chooses to represent/implement exceptions internally).

This flexibility would be especially useful for interop. For example, in GC languages we're very likely to see exceptions like $__java_exc : [(ref $javaref)]. If we make a JS interop for GC that accommodates requests that have already been made regarding exceptions, then the payload of the $__java_exc is already the JS exception that should be caught. So the conversion function for $__java_exc would simply be the coercion from (ref $javaref) to externref that the JS API for GC would enable (just by importing the identity function from JS).

This also seems a lot more portable across hosts. Using an import in order to throw an exception (rather than using the throw instruction) only works well on hosts with exceptions. Plus using imports to throw makes things difficult to optimize (e.g. if you have a throw inside a matching catch you can change it to just a br, but you can't do the same for a call to an imported function). And, again for hosts without exceptions, having the conversion functions makes it much easier for host functions to handle calls to WebAssembly functions as they can just handle (or propagate) the converted externref rather than all the possible cases of exceptions using tags that might not even be exposed.

@aheejin
Copy link
Member

aheejin commented Oct 29, 2021

What I’d like to achieve as a bottom line is

  1. We make a bare wasm throw quick and simple, without mandating it generating stack traces
  2. We make a way with which toolchains can generate stack traces and allow them to be treated in a natural way (e.g. pretty-printed in consoles/devtools, or sending the stack trace back to the server) like a JS Error would do. The toolchain can do that by using an imported function when the user opts in to stack traces.

I think we can achieve these objectives in either case.

  • If WebAssembly.Exception is a subclass of JS Error:
    I think we would do as WebAssembly.Exception is not a subclass of JS Error #183 (comment) suggested: When thrown from a bare wasm throw, it will make Error's fields (name, message, and stack, ..) empty. When a user opts in to stack traces, a toolchain can use an imported function to create WebAssembly.Exception.

  • If WebAssembly.Exception is not a subclass of JS Error:
    An exception thrown from a bare wasm throw will not be a JS Error. This is also what you get when you throw a simple number or string in JS. If a user opts in to stack traces in a toolchain, the toolchain can generate WebAssembly.Exeption and make the object behave like JS Error on the fly in its JS imported function. For example,

WebAssembly.Exception.prototype = Error.prototype;
var e = new WebAssembly.Exception(...);
e.name = "...";
e.message = "...";
e.stack = new Error().stack;
throw e;

Something like this will make the object behave like JS Error when a user opts in.

This patching of the Exception object would work for Emscripten for now (given that mixing Emscripten-generated wasm with other languages is rare) but obviously isn’t ideal. I do think it would make sense to improve interoperability in a follow-up change, and the current proposal wouldn’t prevent that.

@aheejin
Copy link
Member

aheejin commented Oct 29, 2021

@RossTate

But I thought of a refinement of that suggestion which happens to line up with @dschuff's lazy semantics, so hopefully that's an indication that we're converging. When you define a new exception tag, in addition to specifying its type [t*], also specify a function id $func of a function of type [t*] -> [externref]. For the JS API, when a throw $exn reaches a JavaScript catch (e) (or the top of the stack, i.e. is uncaught), this function gets called to convert the payload of the wasm exception into the JS value to be caught.

This provides a lot of flexibility. For example, a module could implement this conversion function by calling an imported function that constructs some subclass of Error, generating the stack trace from that point. Or a module could choose to use an imported function to generate a stack trace at the point the exception was first constructed internally, and then implement this conversion function by constructing some subclass of Error with this preexisting stack trace (which it could retrieve from the payload, or some global, or some table depending on how it chooses to represent/implement exceptions internally).

This flexibility would be especially useful for interop. For example, in GC languages we're very likely to see exceptions like $__java_exc : [(ref $javaref)]. If we make a JS interop for GC that accommodates requests that have already been made regarding exceptions, then the payload of the $__java_exc is already the JS exception that should be caught. So the conversion function for $__java_exc would simply be the coercion from (ref $javaref) to externref that the JS API for GC would enable (just by importing the identity function from JS).

I think this is an interesting new design problem. This may not be relevant only to exceptions; this can be a question of a general conversion interface between languages' reference types and externref of JS Error. Also there can be multiple ways to tackle this problem: We can enable registration of conversion functions between reference types similar to what you suggested, or make a way of designating some user objects as error objects as WebAssembly/gc#235 (comment) suggested.

So I think this warrants more exploration and is too complex a problem to discuss within this issue that discusses a simple yes/no question in JS API. I think a follow-on proposal is a better way to explore this issue more.

This also seems a lot more portable across hosts. Using an import in order to throw an exception (rather than using the throw instruction) only works well on hosts with exceptions.

I think we are talking about JS API here, and JS has throw, so I think it's fine.

Plus using imports to throw makes things difficult to optimize (e.g. if you have a throw inside a matching catch you can change it to just a br, but you can't do the same for a call to an imported function).

In cases where exceptions are for error handling, it is very likely that the actual throw will be inside a language library function, along with other tasks that the language library needs to do. So the throw itself will be very rare, and optimization for it won’t matter. In cases where throws are common as a form of general control flow (where optimization would make a big difference), a toolchain would likely just use the bare throw instruction since it wouldn't want to collect a stack trace.

@Ms2ger
Copy link
Contributor

Ms2ger commented Nov 2, 2021

What I’d like to achieve as a bottom line is

1. We make a bare wasm `throw` quick and simple, without mandating it generating stack traces

This sounds reasonable.

2. We make a way with which toolchains can generate stack traces and allow them to be treated in a natural way (e.g. pretty-printed in consoles/devtools, or sending the stack trace back to the server) like a JS `Error` would do. The toolchain can do that by using an imported function when the user opts in to stack traces.

This doesn't sound so satisfying, but is perhaps fine for an MVP, and we might do something nicer once we have better support for strings in wasm.

WebAssembly.Exception.prototype = Error.prototype;
var e = new WebAssembly.Exception(...);
e.name = "...";
e.message = "...";
e.stack = new Error().stack;
throw e;

I realized this won't work as the jsapi spec is currently written. Consider a JS function jsMain that calls a wasm function wasmThrow that calls a JS function jsThrow with the above code. Since we don't have exnref anymore, "create a host function" will take the tag/payload out of the Exception object as it flows into wasm (so that it can be caught within the wasm module) and construct a new Exception object, so jsMain will not see the object that was created in jsThrow. I see three alternatives:

  • jsThrow just creates a plain Error and wasmThrow can't extract any data from it
  • we drop the special case in "create a host function" and treat all exceptions as opaque objects
  • jsThrow uses a tag that leaves room for the name/message/stack in the payload, and implements name/message/stack getters on the prototype of Exception, which check for that tag and extract the result from the payload

@RossTate
Copy link
Contributor

RossTate commented Nov 2, 2021

I think a follow-on proposal is a better way to explore this issue more.

Okay, I'll get that process started.

"create a host function" will take the tag/payload out of the Exception object as it flows into wasm (so that it can be caught within the wasm module) and construct a new Exception object

The original exception object is not dropped by the JS API. Rather, catch $tag only catches JS exceptions that are subclasses of the WebAssembly.Exception class that have tag $tag. At that point, it extracts the payload and hands it to wasm code while storing the original exception. Inside the catch $tag body, the rethrow instruction throws that original exception on the stack, whereas throw $tag creates a new exception object (possibly with the same payload as the original).

@Ms2ger
Copy link
Contributor

Ms2ger commented Nov 2, 2021

I think a follow-on proposal is a better way to explore this issue more.

Okay, I'll get that process started.

"create a host function" will take the tag/payload out of the Exception object as it flows into wasm (so that it can be caught within the wasm module) and construct a new Exception object

The original exception object is not dropped by the JS API. Rather, catch $tag only catches JS exceptions that are subclasses of the WebAssembly.Exception class that have tag $tag. At that point, it extracts the payload and hands it to wasm code while storing the original exception. Inside the catch $tag body, the rethrow instruction throws that original exception on the stack, whereas throw $tag creates a new exception object (possibly with the same payload as the original).

Your reading is incorrect.

@RossTate
Copy link
Contributor

RossTate commented Nov 2, 2021

Your reading is incorrect.

Can you clarify? This was the explicit rationale I was given for having rethrow inside of catch.

@Ms2ger
Copy link
Contributor

Ms2ger commented Nov 2, 2021

It's simply not what the spec says. I'm not sure how to spec anything that relies on the identity of an exception inside wasm anyway, without exnref.

@RossTate
Copy link
Contributor

RossTate commented Nov 2, 2021

Ah, thanks for the pointer. It seems that the spec has strayed from what @aheejin had communicated before. It also seems that the formal spec cannot express @aheejin's suggested JS API for the reasons you pointed out.

The approach I suggested above works with the formal spec as is. Otherwise we'll have to revise the formal spec (and engine implementations might need to change accordingly). @aheejin, what would you like?

@dschuff
Copy link
Member

dschuff commented Nov 3, 2021

What I’d like to achieve as a bottom line is

  1. We make a bare wasm throw quick and simple, without mandating it generating stack traces
  2. We make a way with which toolchains can generate stack traces and allow them to be treated in a natural way (e.g. pretty-printed in consoles/devtools, or sending the stack trace back to the server) like a JS Error would do. The toolchain can do that by using an imported function when the user opts in to stack traces.

This pretty well sums up what I want too.

This doesn't sound so satisfying, but is perhaps fine for an MVP, and we might do something nicer once we have better support for strings in wasm.

I agree, and I actually also like @RossTate's idea of a conversion callback, which would allow 2) to happen transparently; i.e. user code (JS catch or onerror handlers) could always get an Error even if it originated as a WebAssembly.Exception. For linear-memory wasm at least, I think the callback would have to be a JS function rather than a wasm function though. I think that making it a followup rather than in the MVP is probably fine.

But even when we do that, I do think that we still want to try to get the property that when we throw a WebAssembly.Exception from JS and let it propagate through a wasm function (regardless of whether it is caught and rethrown or not) and back out to JS, the WebAssembly.Exception that comes out has the same properties as the one that went in. That will allow such objects to carry extra information (be it stack traces, messages or whatever else the toolchain wants) much more conveniently. As mentioned upthread, this has always been what we've been assuming in our discussions. In fact, both Firefox's and Chrome's implementations already have this property (contrary to the current version of the JS spec)! There must be some way to spec it.
How much that would be in the JS API vs the core spec seems tricky, because you'd want this property to hold even if there are no catch instructions in the wasm at all. I wonder if we can cache the object somewhere in between, e.g. in create a host function step 9.1, cache v somewhere, and then in call an exported function 11.2 (where the exception propagates back to JS), pull it out again rather than creating a new Exception object. I suspect that the engines are probably using something like an address (i.e. like externaddr) for this purpose. I wonder if it would be possible to give exceptions such an address as well.

@Ms2ger
Copy link
Contributor

Ms2ger commented Nov 4, 2021

I guess the most straightforward way to spec it would be to attach an externref to the core exception, and make sure it's propagated by catch; rethrow, but not expose it to wasm code. Then hosts that don't need it, should be able to avoid storing it at all. Trying to keep this feature out of the core spec entirely sounds like it would be quite a mess.

@RossTate
Copy link
Contributor

RossTate commented Nov 4, 2021

There's another problem related to this issue. C++ has a catch (...) that can catch foreign exceptions, and various languages compiling to or interoperating with JS have ways to catch foreign/JS exceptions as well. (For example, catch (err : Throwable) in Kotlin will catch all exceptions, including JS exceptions. In the Kotlin-to-JS transpiler, it's translated to simply catch (e) in JS.) The expectation for these systems in the JS embedder is inevitably going to be that these constructs catch foreign wasm exceptions as well. Unfortunately, the current JS API strategy does not facilitate this at all; if anything it hinders it by making it impossible to catch foreign wasm exceptions (even though it's possible to catch non-wasm JS exceptions).

What I expect these systems will inevitably do is what the libc++abi you're using expects you do to: convert foreign exceptions into internal exceptions (using some form of wrapping/proxies). For example, Kotlin will have some (unexposed) subclass of Throwable that has an externref field storing the foreign JS/wasm exception, and $__kotlin_exn's callback for converting a ref $Throwable to an externref will check to see if the throwable belongs to this subclass and unwrap it. For libc++abi, the expectation is that you use __cxa_allocate_exception to allocate a (foreign) exception object, initialize it (e.g. with some field that identifies the index of the externref of the foreign exception in some table), and then use _Unwind_RaiseException to internally propagate the exception up the stack. Doing so will make it so that the current compilation of catch (...), which only handles the $__cpp_exception, will properly handle foreign exceptions (including rethrowing via throw;) without any change.

So the JS API that people looking for good interop with JS are going to want is one that makes it easy to do this conversion. With the JS API strategy y'all are using, they'll have to wrap every import on the JS side with a function that catches exceptions and rethrows them as the internal exception. Except they'll need to define these wrappers before the exception tag to use has been defined (since that's created during instantiation). Thus they'll have to jump through many hoops to handle what is likely the common use case (especially if they want to use EcmaScript Modules). A more broad way to convert exceptions in a custom manner would be much more usable. There are a few ways I can see that being done, but I'll table that for now.

Doing so will make both WebAssembly.Exception and implicit information unnecessary; in fact, they'd likely become burdensome to use. For example, the present tooling for C++ is unable to preserve the stack trace of the original exception when using throw;. The easiest way to fix this is to associate an externref of the stack trace with each exception (again, via some table), in which case the existing implementation of __cxa_rethrow will preserve this stack trace. That requires reasoning about the stack trace explicitly rather than implicitly.

@dschuff
Copy link
Member

dschuff commented Nov 5, 2021

I guess the most straightforward way to spec it would be to attach an externref to the core exception, and make sure it's propagated by catch; rethrow, but not expose it to wasm code. Then hosts that don't need it, should be able to avoid storing it at all. Trying to keep this feature out of the core spec entirely sounds like it would be quite a mess.

I think this would make perfect sense actually. It matches the way we have been thinking of exceptions the entire time (i.e. exceptions have an identity, which is preserved by rethrow), and it matches the way that exceptions are implemented in engines in practice.

Unfortunately, the current JS API strategy does not facilitate this at all; if anything it hinders it by making it impossible to catch foreign wasm exceptions (even though it's possible to catch non-wasm JS exceptions).

A foreign wasm exception can be caught in JS, just as it can in wasm. If the tag isn't exported to JS, then it will effectively be opaque, just like catching a foreign exception in wasm. (Assuming you stick to the documented interface and don't try to introspect the properties on the WebAssembly.Exception object that you catch).

So the JS API that people looking for good interop with JS are going to want is one that makes it easy to do this conversion.

As I said above, I do actually like the idea of facilitating these conversions. I think we should explore this soon. We'll have to balance making sure we think it will work well with wasm GC (predicting the future, since it's still fairly early for that spec), with getting something out soon that can be useful with linear memory programs.

In the meantime I think there is still value in an "MVP" API that has the basic reflections of exception tags, identity, and payloads of non-foreign exceptions (i.e. those exported/imported tags) into JS. I don't think it prevents us from making it much nicer and more transparent in the future.

@RossTate
Copy link
Contributor

RossTate commented Nov 5, 2021

@dschuff Let me clarify the intent of those two quotes:

  1. I understand that it's possible to catch foreign wasm exceptions in JS. What I meant was that it's impossible to catch foreign wasm exceptions in wasm even though it's possible to catch JS exceptions in wasm with your JS API.
  2. The "this" in the second quote is referring to conversions from exceptions in JS to exceptions in wasm, whereas the conversion you're referring to that I suggested earlier is for converting from exceptions in wasm to exceptions in JS.

To see why each of these are important to various languages compiling to the web, consider the following C++ program:

void foo() {
    try {
        coin_flip() ? javascript_function() : local_function();
    } catch (...) {
        rethrow_non_std_exception();
    }
}
void rethrow_non_std_exception() { // requirement: only called from within a catch
    try {
        throw;
    } catch (const std::exception& e) {}
}

With the present tooling, exceptions thrown by javascript_function will bypass the catch (...).

With my suggestion of custom js-to-wasm exception conversion, one can add support for catching foreign exceptions (and rethrowing them) with just a few focused changes. In fact, one can even add a dynamically mutable configuration option for switching between different semantics for foreign exceptions: bypass unwinding altogether, unwind but do not catch, and unwind and catch.

With the current JS API for js-to-wasm exception conversion, you have to make significant changes to the entire infrastructure. For example, you cannot simply import and use the $__js_exception : [externref] tag because that will not catch exceptions thrown from within JavaScript functions that happen to have percolated up from WebAssembly programs, as those get converted to wasm exceptions with tags you have no access to (issue 1 at the beginning of this post). So instead the tooling has to wrap every would-be-imported JS function (issue 2 at the beginning of this post) with a function that catches JS exceptions and rethrows them as the instance's $__cpp_exception after suitably wrapping them. But you don't have access to the instance's $__cpp_exception tag before you create it by instantiating the module with the import wrappers you are creating; thus you need infrastructure to tie the knot..

@dschuff
Copy link
Member

dschuff commented Nov 5, 2021

OK thanks, that does make it clearer.
I actually (still) agree with you that VM-assisted custom conversion in both directions seems like a good idea. I expect we'd use it in Emscripten once we have it, even in the absence of foreign wasm exceptions. I'm not sure whether you're trying to make the case that no MVP JS API can be useful without it, but I don't think I agree with that.

Actually, would you mind maybe writing up something about this in a standalone issue? Given that this involves the VM calling user code implicitly (something I don't think we have anywhere in wasm/JS yet?) I would be interested in hearing what VM implementers think about that idea.

[edit: I guess the start function is one place where the VM does implicitly call into user-generated code. But even in that case it's done inside an explicit API call, rather than during unwinding]

@RossTate
Copy link
Contributor

RossTate commented Nov 6, 2021

Can do (next week 😄)! Thanks.

@aheejin
Copy link
Member

aheejin commented Nov 6, 2021

As I said in the previous comment, I think reference conversion API is a whole new design issue, which would better be solved in a separate proposal. I don't think the current MVP proposal precludes potential future improvements.

I also asked that we discuss different topics in different issues. This issue was discussing a very narrow JS API problem, so I'm not sure if this post is a good place for a whole new conversion API discussion. Can we discuss this topic in another issue? The new repo for the conversion API proposal will even be a better place.

@Ms2ger
Copy link
Contributor

Ms2ger commented Nov 8, 2021

Filed #189 for preserving the JS exception identity; please file issues if there's other changes to be proposed.

@RossTate
Copy link
Contributor

RossTate commented Nov 8, 2021

And I filed #190 so that we'd have a separate space to discuss custom conversions, as requested.

@dschuff
Copy link
Member

dschuff commented Nov 9, 2021

Great!
Just in case it wasn't clear, I agree that #190 shouldn't block moving the main proposal to phase 4. If we can keep it as a JS-API-only change, we can also maybe even advance it as a spec PR rather than having to go with a separate repo (as we are with WebAssembly/spec#1300)

@RossTate
Copy link
Contributor

RossTate commented Nov 9, 2021

Got it. And, to also make sure we're all on the same page, once such an extension to the JS API for EH is made, none of the most complex instructions—catch_all/delegate/rethrow—are likely to be used by anyone anymore (including C++), though of course web engines will have to continue to support them for backwards-compatibility purposes.

@dschuff
Copy link
Member

dschuff commented Nov 9, 2021

Language developers will use whatever instructions make their lives easier. We at least don't have any plans to change C++ codegen even in the presence of this extension.

@dtig dtig added this to the / milestone Nov 16, 2021
@eqrion
Copy link
Contributor

eqrion commented Nov 17, 2021

Okay, trying to get caught up on this discussion. Here are the important details I'm seeing.

  • Identity of thrown exceptions in wasm is not properly preserved, but this is being resolved
  • We'd like for a bare wasm throw to not collect stack traces, keeping it fast
  • We'd like for toolchains to have a method for opt-ing into collecting stack traces
  • We cannot leave whether or not WebAssembly.Exception inherits from Error unspecified
  • The .stack string property of Error is non-standard, but present on all engines
  • (my addition) Error has additional properties besides .stack such as .lineNumber/.cause (some standard/some non-standard). If we inherit from Error we need to decide their behavior as well
  • V8 has a non-standard API for limiting stack trace collection, Error.stackTraceLimit
  • (my addition) Firefox Devtools/Spidermonkey special case thrown Error objects to display rich stack traces. It would be nice to re-use this

All of this being said, how would this proposal sound?

  1. Make WebAssembly.Exception inherit from Error.
  2. Suppress the generation of .stack/.lineNumber/.cause when created in a wasm throw. They are left as 'zero values' (empty string or 0).
  3. Use the existing VM mechanisms (if any) for deciding to create a stack trace when the WebAssembly.Exception JS-API constructor is used.
    a. For V8, this would follow Error.stackTraceLimit.
    b. For SM (JSC?), this would always generate a stack trace.
    c. .lineNumber/.fileName are collected as normal unconditionally?
    d. This does not add a boolean flag to the constructor, as its semantics are dependent on a non-standard feature and so it's hard to specify a meaning to it.
  4. Toolchains would be expected to import a JS function for throwing when they want to opt-in to stack tracing, using the JS-API.

A second option is to just do nothing, leaving WebAssembly.Exception as not an Error. Then toolchains could manually construct WebAssembly.Exception and add a .stack property using (new Error()).stack. This would not benefit from any special handling present in developer tools, and so I think it's less of a preference.

@dschuff
Copy link
Member

dschuff commented Nov 19, 2021

TL:DR; I'd be OK with this proposal if other folks would be.

Okay, trying to get caught up on this discussion. Here are the important details I'm seeing.

  • Identity of thrown exceptions in wasm is not properly preserved, but this is being resolved
  • We'd like for a bare wasm throw to not collect stack traces, keeping it fast
  • We'd like for toolchains to have a method for opt-ing into collecting stack traces
  • We cannot leave whether or not WebAssembly.Exception inherits from Error unspecified
  • The .stack string property of Error is non-standard, but present on all engines
  • (my addition) Error has additional properties besides .stack such as .lineNumber/.cause (some standard/some non-standard). If we inherit from Error we need to decide their behavior as well
  • V8 has a non-standard API for limiting stack trace collection, Error.stackTraceLimit
  • (my addition) Firefox Devtools/Spidermonkey special case thrown Error objects to display rich stack traces. It would be nice to re-use this

I think this is all correct. I think V8 has a similar mechanism for displaying rich stack traces when Errors are thrown.

All of this being said, how would this proposal sound?

  1. Make WebAssembly.Exception inherit from Error.
  2. Suppress the generation of .stack/.lineNumber/.cause when created in a wasm throw. They are left as 'zero values' (empty string or 0).
  3. Use the existing VM mechanisms (if any) for deciding to create a stack trace when the WebAssembly.Exception JS-API constructor is used.
    a. For V8, this would follow Error.stackTraceLimit.
    b. For SM (JSC?), this would always generate a stack trace.
    c. .lineNumber/.fileName are collected as normal unconditionally?
    d. This does not add a boolean flag to the constructor, as its semantics are dependent on a non-standard feature and so it's hard to specify a meaning to it.
  4. Toolchains would be expected to import a JS function for throwing when they want to opt-in to stack tracing, using the JS-API.

This is close to what I had in mind above. I think I have a mild preference for using some kind of flag in the JS-API constructor, in order to have a cross-platform way to control the stack trace behavior. I agree that it could be tricky to specify given that the underlying semantics are not specified; the language might end up being a little weasel-y like the current spec for how to spell stack frames. But it would move us a small step in the right direction toward more consistency across engines.
But your version would be OK too; it would certainly be a bit simpler, and you could of course say that one cross-platform way to opt out of stack tracing in any case would be to just call into a wasm function with a throw instruction.

A second option is to just do nothing, leaving WebAssembly.Exception as not an Error. Then toolchains could manually construct WebAssembly.Exception and add a .stack property using (new Error()).stack. This would not benefit from any special handling present in developer tools, and so I think it's less of a preference.

Yes, the advantage of this (aside from an even simpler JS spec) would be that we wouldn't end up with another kind of Error objects that lack stack traces. This kind-of feels nicer to me, but it's not really clear what the concrete advantages might be.
There are 2 disadvantages. One is the one you mentioned, that devtools (and shells like nodejs) treat Error objects specially, and they wouldn't recognize WebAssembly.Exception objects. I actually think it would not be too difficult to fix that, such that if an exception object had a stack trace or similar fields that are specially handled, it could get the same treatment that Error does. The other disadvantage is for user-written code, such as try/catch wrappers or onerror handlers, for example:

try {
  call_some_js_and_wasm();
} catch (err) {
  send_stacktrace_to_server(err.stack);
}

If a toolchain appended a stack trace to its WebAssembly.Exception objects (which I expect e.g. Emscripten would), then this would actually just work. However the difference is still observable, so the following wouldn't work without changes:

try {
  call_some_js_and_wasm();
} catch (err) {
  if (err instanceof Error) {
    send_stacktrace_to_server(err.stack);
  } else {
    do_something_else();
  }
}

This could easily be fixed by the developer, but of course doing nothing is easier than having to do something.
This could also be made fully-transparent in a follow-on proposal such as #190 if we wanted to do that.

@aheejin
Copy link
Member

aheejin commented Nov 20, 2021

Thanks @eqrion and @dschuff for the summary! So assuming we resolve the identity issue (#189), to sum up the three alternatives:

  1. Make WebAssembly.Exception a subclass of JS Error and does not make includeStackTrace parameter (what @eqrion advocated)
    Pros: Nice Error handling in devtools. Simpler to spec, because includeStackTrace depends on non-standard features
    Cons: No way not to generate stack traces using JS API in some engines, like JSC

  2. Make WebAssembly.Exception a subclass of JS Error and make includeStackTrace parameter (what @dschuff advocated)
    Pros: Nice Error handling in devtools. includeStackTrace enables every engine to turn on and off stack traces
    Cons: includeStackTrace depends on non-standard features

  3. Make WebAssembly.Exception not a subclass of JS Error
    Pros: Simple JS API spec.
    Cons: Tools have to do some monkey patching. Some syntax, like, err instanceof Error wouldn't work.

I don't have strong preferences, but if I have to pick one, I would like 2, just because with 1, in some engines, we may not be able to disable stack traces when creating an exception from JS API. Of course, we can always create a function that contains a bare wasm throw, but JS API not having the full capability of wasm throw worries me a little.

Also I'm not sure how much of a trouble would it be that includeStackTrace parameter depends on non-standard features. Stack trace itself is non-normative, so can't we spec like

  • If includeStackTrace == false, we make WebAssembly.Exception not generate any stack traces and be as efficient as a bare wasm throw
  • If includeStackTrace == true, we allow WebAssembly.Exception generate stack traces, but for VMs that don't support stack traces, it's not mandatory

@eqrion
Copy link
Contributor

eqrion commented Dec 1, 2021

Sorry for the delay in continuing this discussion. I solicited some more opinions internally and received some more thoughts.

On further thought, making WA.Exception a subclass of Error is problematic as not all users of this proposal may consider their uses 'errors', and making WA.Exception subclass Error would imply that. For example, a language using this for control flow would not want their exception object to be an Error. One hypothetical issue could be a JS middleware looking for instanceof Error and getting overwhelmed by actually non-erroneous usage of WA.Exception.

On the philosophical level, there's a symmetry between JS an Wasm that would be nice to have. JS allows you to throw any value and it's not always an 'error' (represented by Error). By making WA.Exception an Error, we would not allow wasm to throw a value that's not an 'error'.

That all being said, I would now prefer @aheejin's (3) option (no subclassing). Thinking about stack tracing more, I think it would likely be fine to have a flag in the constructor for WA.Exception that opts-in to stack tracing. We could then specify a builtin but optional .stack property to WA.Exception. Spec text could say that it has the same format as Error.stack if that exists.

@aheejin
Copy link
Member

aheejin commented Dec 21, 2021

Thanks @eqrion! Sorry for the delay too. I think what you said makes sense; it can be risky to assume every WebAssembly.Exception is an Error. If we assume we choose (3), we only have one thing to choose: whether to make the boolean argument to the constructor of WebAssembly.Exception, which signals whether we include stack traces or not.

If we make that option, I think the option should be false by default. If the option is on, the VM has an option of populating some JS Error properties like stack, name, or message. I'm not sure if we can mandate them even if this option is true, given that stack is a non-standard field even in JS Error. Also there's a possibility that non-web VMs may not support stack traces. In this case, I can they simply ignore the option, or should they error out? I think ignoring is fine given that some of those fields are non-standardized anyway.

In case we don't make the boolean option for stack traces, it is entirely up to the toolchain to create and populate those fields when it thinks it is appropriate. For example, Emscripten, which is a C/C++ compiler, can choose to populate the fields, because in C++ exceptions mean errors. But possibly in toolchains for other languages, they choose not to do that in case a language uses Wasm exceptions simply for control flow transfers and not errors.

I don't have a strong preference at this point. Please let me know if you incline towards either.

@eqrion
Copy link
Contributor

eqrion commented Dec 21, 2021

Thanks @eqrion! Sorry for the delay too. I think what you said makes sense; it can be risky to assume very WebAssembly.Exception is an Error. If we assume we choose (3), we only have one thing to choose: whether to make the boolean argument to the constructor of WebAssembly.Exception, which signals whether we include stack traces or not.

If we make that option, I think the option should be false by default. If the option is on, the VM has an option of populating some JS Error properties like stack, name, or message. I'm not sure if we can mandate them even if this option is true, given that stack is a non-standard field even in JS Error.

Yes, I think a boolean argument to the WebAssembly.Exception constructor for traceStack that is false by default is our best option. I think we could specify it reasonably well as follows:

[LegacyNamespace=WebAssembly, Exposed=(Window,Worker,Worklet)]
interface Exception {
  constructor(Tag exceptionTag, sequence<any> payload, optional boolean traceStack);

  readonly attribute (DOMString or undefined) stack;

  any getArg(Tag exceptionTag, unsigned long index);
  boolean is(Tag exceptionTag);
};

traceStack defaults to false if not provided. If traceStack is true and the implementation supports the Error.stack attribute, then the Exception.stack attribute must be the same value as (new Error()).stack (the wording here could be relaxed if need be). Otherwise the stack attribute must be undefined.

Additionally, I think we should exclude the non-stack properties like message and name for now.

Also there's a possibility that non-web VMs may not support stack traces. In this case, I can they simply ignore the option, or should they error out? I think ignoring is fine given that some of those fields are non-standardized anyway.

I don't think we need to worry about non-web VMs here, as this is just a JS-API feature and so I think we can assume a web VM.

@eqrion
Copy link
Contributor

eqrion commented Dec 30, 2021

Refining the WebIDL snippet further from feedback, we should avoid a positional optional boolean parameter to prevent a future extension hazard.

dictionary ExceptionOptions {
  optional boolean traceStack;
};
...
constructor(Tag exceptionTag, sequence<any> payload, optional ExceptionOptions options);

aheejin added a commit to aheejin/exception-handling that referenced this issue Feb 4, 2022
This updates the explainer on
- We add stack trace option to `Exception` class in the JS API
- `Exception` can contain an optional `externref` to carry a stack trace
- `Exception` is not a subclass of JS `Error`

Addresses WebAssembly#183 and WebAssembly#189.
aheejin added a commit to aheejin/exception-handling that referenced this issue Feb 4, 2022
This updates the explainer on
- We add stack trace option to `Exception` class in the JS API
- `Exception` can contain an optional `externref` to carry a stack trace
- `Exception` is not a subclass of JS `Error`

The WebIDL syntax is suggested by @eqrion in
WebAssembly#183 (comment)
and
WebAssembly#183 (comment).

Addresses WebAssembly#183 and WebAssembly#189.
Ms2ger pushed a commit that referenced this issue Feb 10, 2022
This updates the explainer on
- We add stack trace option to `Exception` class in the JS API
- `Exception` can contain an optional `externref` to carry a stack trace
- `Exception` is not a subclass of JS `Error`

The WebIDL syntax is suggested by @eqrion in
#183 (comment)
and
#183 (comment).

Addresses #183 and #189.
@aheejin
Copy link
Member

aheejin commented Feb 16, 2022

Since we updated the explainer in #197 and #203, will close this.

@aheejin aheejin closed this as completed Feb 16, 2022
@dtig dtig removed this from the / milestone Jun 14, 2022
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

No branches or pull requests

10 participants