Skip to content

[js-api] Add description of interface additions to overview document #154

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

Merged
merged 3 commits into from
Jun 24, 2021

Conversation

takikawa
Copy link
Collaborator

Based on the discussion in #150, this PR adds a description of new proposed JS API interfaces for exceptions to the overview document.

This is a draft and I'm open to any suggestions. I commented in #150 about what other points we might adjust or discuss.

The overview is also based on @Ms2ger's formal spec PR #86.

Copy link
Member

@aheejin aheejin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for doing this! Sorry for the delayed reply. This looks good to me, with one thing to consider, as I also commented in #150 (comment): I'm OK with Exception / RuntimeException names here, but should we rename event to exception altogether in the core spec? What do you think, @rossberg @ioannad ?


The `WebAssembly.Exception` class represents an exception defined in the event section and exported from a WebAssembly module. It allows querying the type of an exception following the [JS type reflection proposal](https://github.com/WebAssembly/js-types/blob/master/proposals/js-types/Overview.md). Constructing an instance of `Exception` creates a fresh exception tag, and the new exception can be passed to a WebAssembly module as an import.

The `WebAssembly.RuntimeException` class represents an exception thrown from WebAssembly, or an exception that is constructed in JavaScript and is to be thrown to a WebAssembly exception handler. The `RuntimeException` constructor accepts an `Exception` argument and a sequence of arguments for the exception's data fields. The `Exception` argument determines the exception tag to use. The data field arguments must match the types specified by the `Exception`'s type. The `is` method can be used to query if the `RuntimeException` matches a given `Exception`'s exception tag. The `getArg` method allows access to the data fields of a `RuntimeException` if an `Exception` is passed with a matching exception tag. This last check ensures that without access to a WebAssembly module's exported exception, the associated data fields cannot be read.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The getArg method allows access to the data fields of a RuntimeException if an Exception is passed with a matching exception tag. This last check ensures that without access to a WebAssembly module's exported exception, the associated data fields cannot be read.

Why is this check necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the issue is that it would give JS capabilities that the Wasm semantics is not intended to have. Specifically Wasm code cannot ever extract the values for a thrown exception if it doesn't have the tag for it. But if JS could do this via getArg then Wasm could also do it by calling out to JS.

(original discussion of this check is here #109 (comment))

@ioannad
Copy link
Collaborator

ioannad commented Jun 3, 2021

@aheejin

Thanks a lot for doing this! Sorry for the delayed reply. This looks good to me, with one thing to consider, as I also commented in #150 (comment): I'm OK with Exception / RuntimeException names here, but should we rename event to exception altogether in the core spec? What do you think, @rossberg @ioannad ?

Sounds good to me! Since we have the attribute-bit in the binary format of an exception (event), in case we do add other events in future proposals, I think it's simpler to just use the term "exception" in this proposal.

@aheejin
Copy link
Member

aheejin commented Jun 3, 2021

@aheejin

Thanks a lot for doing this! Sorry for the delayed reply. This looks good to me, with one thing to consider, as I also commented in #150 (comment): I'm OK with Exception / RuntimeException names here, but should we rename event to exception altogether in the core spec? What do you think, @rossberg @ioannad ?

Sounds good to me! Since we have the attribute-bit in the binary format of an exception (event), in case we do add other events in future proposals, I think it's simpler to just use the term "exception" in this proposal.

I'm not sure if I understand. Are we still planning to add other kinds of events to the event section? If so and the attribute bit signals the intent, we should consider rather keeping the name 'event', because exception can be considered as a subtype of event but not vice versa.

What I'm saying is, we would better be consistent - if we are quite sure we are not going to add other kinds of events to the current event section, we can rename it as the exception section and use the same names in the JS API. If we plan to add other kinds of events in the event section or are not sure, we may need to keep the name, and using 'exception' in the JS API becomes weird, because, is that API going to only handle exceptions and not other kinds of potential future events?

@aheejin aheejin mentioned this pull request Jun 7, 2021
Ms2ger pushed a commit to Ms2ger/exception-handling that referenced this pull request Jun 9, 2021
The encoding of the table index in binary-leb128.wast is incorrect with the bulk-memory extensions, see WebAssembly/bulk-memory-operations#153. I saw and fixed the issue first in the reference types proposal (see WebAssembly/reference-types#95), but apparently it also exists here.
@takikawa
Copy link
Collaborator Author

I updated this PR to match the proposal in #161 for tags.

Copy link
Member

@aheejin aheejin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@aheejin aheejin mentioned this pull request Jun 22, 2021
@Ms2ger Ms2ger merged commit 059df76 into WebAssembly:master Jun 24, 2021
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.

4 participants