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

Details of the JS API for exception handling #150

Closed
takikawa opened this issue Mar 31, 2021 · 11 comments
Closed

Details of the JS API for exception handling #150

takikawa opened this issue Mar 31, 2021 · 11 comments

Comments

@takikawa
Copy link
Collaborator

takikawa commented Mar 31, 2021

Now that the semantics for the current iteration of the proposal are fairly solid and two browser implementations are underway, I was wondering if this is a good time to figure out the details for the JS API for exceptions.

@Ms2ger has written up draft JS API spec text which provides a good baseline to expand on. This draft was made during the previous iteration of the semantics, so it needs an update to remove exnref. But it already describes WebAssembly.Exception and WebAssembly.RuntimeException that are discussed below and additional API features can be incorporated into it.

Here are some of the aspects that the final JS API could/should likely provide, along with some design questions on how these aspects should work:

  • Provide an interface for exported exception types such as WebAssembly.Exception.
    • Previous discussions have generally assumed this should exist, and be called WA.Exception.
    • Should it support the type reflection API proposal? i.e., a type() method on exceptions.
    • Should it be possible to construct these in JS (given a function type or just a list of parameters from the type reflection API), to create a unique exception tag on the JS side? (which a Wasm module could import)
  • Provide an interface for Wasm exceptions caught in JS such as WebAssembly.RuntimeException.
    • A previous discussion reached a consensus to call this WebAssembly.RuntimeException and to keep it separate from WebAssembly.RuntimeError.
    • Should it allow access to exception values? (e.g., values() or getArg() method, using ToJSValue to coerce Wasm values to JS as usual)
    • If values are accessible, it should require a capability (i.e., the exception type export) to access. See this comment for why.
    • As discussed in the link in the previous bullet, should it have a method to let you check the tag against an exception type?
    • Should JS be allowed to construct these? (e.g., given an exception type, and values that would be coerced via ToWasmValue)

In terms of features, IMO it would make sense to allow accessing the values (requiring the exception type for access) and to allow constructing exception objects. Not adding this functionality won't actually prevent JS hosts from doing these operations (both limitations can be hacked around by constructing a new Wasm module) but adding it will make it more convenient.

There was some past discussion here and here on how/whether to allow constructing exception objects in JS, given the values to put in the exception. Ms2ger's draft assumes that the WA.RuntimeException constructor would also take a WA.Exception argument as well. IMO this would make sense to keep in the API in order to extract the tag out of the exception type. Not having this argument would make the constructor pretty useless, as without a tag such exceptions could only be caught by catch_all.

Are there any other aspects of the JS API that should also be discussed?

@lars-t-hansen
Copy link
Contributor

Why would it be 'WA.Exception' and not 'WA.Event', since the module deals in events, and exceptions are just one event class among others? The Event object might then have attribute() and type() accessors.

@ioannad
Copy link
Collaborator

ioannad commented Apr 7, 2021

Why would it be 'WA.Exception' and not 'WA.Event', since the module deals in events, and exceptions are just one event class among others? The Event object might then have attribute() and type() accessors.

@lars-t-hansen, when we were discussing the 2nd proposal formal spec, @rossberg and I agreed that Events should only be a bit in the binary format for future compatibility and that Events should be not mentioned at all in the formal spec (see e.g.this, this, and this comment).

However, that was a very long discussion on many details of the formal spec, and I don't know what @aheejin or others think about this.

@rossberg
Copy link
Member

rossberg commented Apr 7, 2021

Hm, I haven't really thought about the best way to expose these in JS land. I suppose that WA exceptions should also be JS exception, since you can catch them. But will that be true for other future forms of events? Could it be that we'll ultimately need some JS class hierarchy for this?

@tlively
Copy link
Member

tlively commented Apr 7, 2021

Oh interesting, switching to use "Exception" terminology rather than "Event" terminology would be a big shift in my mental model of the proposal. I see the appeal and I'm sure I'd get used to it quickly, though.

@aheejin
Copy link
Member

aheejin commented Apr 29, 2021

Sorry for the late reply. Yes I think we should flesh out the JS API before we proceed to Phase 3. I agree there better be a way to construct an exception from JS and throw it. It is different from wasm instructions though, with which we do creating and throwing at the same time, but wasm exception will surface as a JS exception anyway, I think having a dedicated class for wasm exception and making a way to create it make sense.

The 'exception' I am talking about here is a constructed exception that contains an event tag and its arguments. Someone, I don't remember exactly, suggested to call this ExceptionPackage at some point. I think Event should mean each event entry in the event section. So I'm not sure about what @ioannad and @rossberg suggested. Do you mean we should remove the name 'event' from JS API entirely? Then what should we call each entry in the event section? If we want to remove the name 'event' and change it to 'exception', I think it should be done both in the spec and the JS API level. The thing the previous thread called WA.Exception was the package that contained all arguments. Are you suggesting that?

I'm not sure if we need to define ToWasmValue(WA.Exception) (here Exception meaning the package) though. Given that in wasm we don't reason with exceptions directly, i.e.,throw packs it and catch unpacks it and we don't touch exceptions, I'm not sure if that object should have any counterpart in wasm. If someone tries that anyway, I guess that can be externref, with which wasm can't do anything.

Previous discussions, especially #109, started assuming the existence of exnref and stemmed from there, so I'm not sure how much of it should we refer to at this point. Please let me know if there should be a takeaway from that post.

I imagine with the JS API we can do thing like

let event = new WebAssembly.Event({parameters: ["i32", "f32"]);
let exn = new WebAssembly.Exception(event, [1, 3.5]);
WebAssembly.throw(exn);

I also think it makes sense for Exception to have a method getArg(i).

On @takikawa's original post:
What is the difference between WA.Exception and WA.RuntimeException? (Here WA is an abbreviation for WebAssembly) If Exception here means an object with an event tag and its arguments, why do we need a separate RuntimeException? Do you use Exception as my Event and RuntimeException as my Exception?

@takikawa
Copy link
Collaborator Author

Thanks for the reply @aheejin. :) Some replies inline below:

I think Event should mean each event entry in the event section. So I'm not sure about what @ioannad and @rossberg suggested. Do you mean we should remove the name 'event' from JS API entirely? Then what should we call each entry in the event section? If we want to remove the name 'event' and change it to 'exception', I think it should be done both in the spec and the JS API level. The thing the previous thread called WA.Exception was the package that contained all arguments. Are you suggesting that?

I don't have a strong opinion here, but I think the idea was that since this proposal only introduces exception events and no other event attributes are currently planned, it could make sense to avoid the generalization and use Exception in all cases. Maybe in that situation, future events with attributes other than Exception could be represented with different classes (e.g., a WA.Continuation for a continuation attribute). Is this a correct representation of what @ioannad and @rossberg were discussing for the spec? (prev discussion)

I'm not sure if we need to define ToWasmValue(WA.Exception) (here Exception meaning the package) though.

I agree that this likely isn't needed in the current proposal. And in particular the WA.RuntimeException I mentioned above could only be passed into Wasm as externref.

I imagine with the JS API we can do thing like

let event = new WebAssembly.Event({parameters: ["i32", "f32"]);
let exn = new WebAssembly.Exception(event, [1, 3.5]);
WebAssembly.throw(exn);

I think this makes sense. I had left out a WA.throw in my sketch above because I was thinking we could use JS throw, is there a use case for having this be a function in the API?

On @takikawa's original post:
What is the difference between WA.Exception and WA.RuntimeException? (Here WA is an abbreviation for WebAssembly) If Exception here means an object with an event tag and its arguments, why do we need a separate RuntimeException? Do you use Exception as my Event and RuntimeException as my Exception?

Yes I agree with your last sentence. The WA.Exception is representing the event/exception type and WA.RuntimeException represents the caught exception package, or a new exception package that you can throw to Wasm. I used some of the names I saw in the issues I linked and in Ms2ger's draft, but I'm not tied to these specific ones.

BTW: I have some WIP patches that implement the API sketch above (without type reflection) for SpiderMonkey in this bug. If you click the attachment links you can also find JS/Wasm test cases that demonstrate how it might work.

@aheejin
Copy link
Member

aheejin commented Apr 30, 2021

Thanks for the reply @aheejin. :) Some replies inline below:

I think Event should mean each event entry in the event section. So I'm not sure about what @ioannad and @rossberg suggested. Do you mean we should remove the name 'event' from JS API entirely? Then what should we call each entry in the event section? If we want to remove the name 'event' and change it to 'exception', I think it should be done both in the spec and the JS API level. The thing the previous thread called WA.Exception was the package that contained all arguments. Are you suggesting that?

I don't have a strong opinion here, but I think the idea was that since this proposal only introduces exception events and no other event attributes are currently planned, it could make sense to avoid the generalization and use Exception in all cases. Maybe in that situation, future events with attributes other than Exception could be represented with different classes (e.g., a WA.Continuation for a continuation attribute). Is this a correct representation of what @ioannad and @rossberg were discussing for the spec? (prev discussion)

I'm not very opinionated on this. I think we can make it back to Exception. But I think it has to be consistent in the core spec and the JS API at least. I would like t avoid calling the same thing with two different names in the core spec and the JS API.

Also, if we decide to change the current Event to Exception, we should come up with a new name that represents an object that contains an event/exception tag and all arguments. Maybe RuntimeException or ExceptionPackage or something.

I imagine with the JS API we can do thing like

let event = new WebAssembly.Event({parameters: ["i32", "f32"]);
let exn = new WebAssembly.Exception(event, [1, 3.5]);
WebAssembly.throw(exn);

I think this makes sense. I had left out a WA.throw in my sketch above because I was thinking we could use JS throw, is there a use case for having this be a function in the API?

Oh, I think you are right. Maybe we don't need to add throw to our JS API.

BTW: I have some WIP patches that implement the API sketch above (without type reflection) for SpiderMonkey in this bug. If you click the attachment links you can also find JS/Wasm test cases that demonstrate how it might work.

Great that it is already in progress in FireFox!


So to sum up,

  • What should we name each of a. each entry in the event section (with a type index), b. the exception object that contains a tag and arguments? I previously assumed each of them as Event / Exception, and the names @takikawa is working on are Exception / RuntimeException. I'm OK with either, but I think we should be consistent on the core spec and the JS API on what to call the former.

  • Do we have anything else to decide?

@takikawa
Copy link
Collaborator Author

Sorry for the lack of progress/update on my end here, in an effort to help move the discussion forward I made a PR in #154 that adds text to the overview document taking into account the discussion here and @aheejin's summary and code example.

From Heejin:

Do we have anything else to decide?

Aside from the naming (BTW I used the names Exception/RuntimeException in PR #154 but I'd be happy to change them to match the consensus choice whether it's Event/Exception or something else), the things I put in the PR I don't think I saw anyone explicitly agree to are:

  • is method in the thrown exception class that lets you check if it matches the type (complements getArg)
  • type method in the exception type class for type reflection.

(so if anyone has any thoughts for/against those, that'd be great)

Also @rossberg had mentioned that maybe Wasm exceptions should also be JS exceptions. My concrete interpretation of this was: should the thrown exception class be a subclass of JS Error? Any thoughts on this? So far I have not included this in the overview PR.

@rossberg
Copy link
Member

#154 looks good to me. I'm actually on the fence regarding making WA.RuntimeException a subclass of Error and would be fine leaving it as is.

@ioannad
Copy link
Collaborator

ioannad commented Jun 3, 2021

Apologies I missed the @-mentions in this issue!

I agree with the original suggestion that there is no real reason to keep the term "event" except as a bit in the binary format in case we do add events in the future.

@aheejin
Copy link
Member

aheejin commented Jun 25, 2021

Closed by #154.

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

6 participants