[JS API] Throw useful exceptions on parse errors#8264
[JS API] Throw useful exceptions on parse errors#8264kripken wants to merge 2 commits intoWebAssembly:mainfrom
Conversation
spotandjake
left a comment
There was a problem hiding this comment.
This looks great to me.
| @@ -3264,7 +3285,7 @@ Object.defineProperty(Module, 'readBinary', { writable: true }); | |||
| Module['readBinary'] = function(data) { | |||
There was a problem hiding this comment.
Just a nit but with #8122 I started trying to add some documentation with jsdoc how would we feel about continuing that pattern when possible?
| Module['readBinary'] = function(data) { | |
| /** | |
| * Creates a module from binary data. | |
| * | |
| * @param {Uint8Array} data - A Uint8Array containing a valid WebAssembly binary. | |
| * | |
| * @return {Module} The constructed wasm module. | |
| * | |
| * @throws {Error} If the input is invalid or parsing fails. | |
| */ | |
| Module['readBinary'] = function(data) { |
There was a problem hiding this comment.
Is this to autogenerate some doc? The comment here seems kind of obvious.
There was a problem hiding this comment.
I assume @spotandjake you are thinking of autogenerating a docs page from the jsdocs comments?
That might be useful, though I'm not sure how much.
| @@ -3273,7 +3294,7 @@ Module['readBinary'] = function(data) { | |||
| Module['parseText'] = function(text) { | |||
There was a problem hiding this comment.
Same as above:
| Module['parseText'] = function(text) { | |
| /** | |
| * Creates a module from Binaryen's s-expression text format (not official stack-style text format). | |
| * | |
| * @param {string} text - A string containing a WebAssembly text module (Binaryen-style WAT). | |
| * | |
| * @return {Module} The constructed wasm module. | |
| * | |
| * @throws {Error} If the input is invalid or parsing fails. | |
| */ | |
| Module['parseText'] = function(text) { |
|
Emscripten supports printing exception messages when run with |
Yes, this is not focused on the stack trace in the exception. This is about being able to catch an exception in JS, and to get the message. Before this PR, a parse error just shut down the entire application, with no useful message for the code calling the binaryen.js library. |
wasm-opt will just fatally error on invalid module inputs, but binaryen.js
is a library and users want to get something they can handle, and see
the actual error. This PR throws a C++ exception instead, and converts
it on the JS side.
Fixes #8256