ghost
commented
2 years ago Tagging subscribers to 'arch-wasm': @lewing See info in area-owners.md if you want to be subscribed.
Issue Details
We need to overhaul the way we do assertions and error handling in the bindings. Right now we have a mix of manual if->throw and the new ```mono_assert``` TS function, along with some parts where C will assert or C# will throw exceptions, and there isn't a ton of consistency in message formats, etc. It can also be awkward to properly format messages without performance issues. @pavelsavara recently wired up a mechanism in the build tooling so that uses of ```mono_assert``` will have the exception message inlined, preventing the normal performance overhead, so that's a start. There are still some other criteria we want to satisfy with any final design: - It should be possible to localize error messages - Each distinct type of error should have a consistent identifier (ERRnnnn etc) that people can use in web searches - Error messages should be informative: if an argument is out of range, specify the argument name, etc - Errors should indicate where they're coming from and whether the problem is under the user's direct control - i.e. did I pass bad data into blazor/wasm, or did the heap get corrupted? - Having plentiful assertions and detailed error messages should not cause performance overhead I believe the solution is to define a new ```errors.ts``` file that exports a function for each unique error message, and you invoke the function to throw that error, passing in any relevant values. This puts all the error messages in one place if we decide to localize them later, and it makes it easy to assign them unique identifiers at a glance. The messages having unique identifiers also means that you can search the whole codebase for a given error using that ID instead of merely hoping that every scenario used the same error text. I haven't had a chance to prototype it yet, but it would look something like this: ```typescript export var errors = { // ... assembly_not_found: (assembly_name) => throw new Error(`WASM003: Could not find an assembly named '{assembly_name}'.`), // ... }; ``` and then a call site would look like: ```typescript const asm = cwraps.mono_wasm_assembly_load(assembly); if (!asm) errors.assembly_not_found(assembly); ``` This approach would also allow us to customize error handling on a per-error basis (for example, updating a specific error to write a log message before throwing).
| Author: | kg |
|---|---|
| Assignees: | - |
| Labels: | `design-discussion`, `arch-wasm`, `area-System.Runtime.InteropServices.JavaScript` |
| Milestone: | Future |
pavelsavara
We need to overhaul the way we do assertions and error handling in the bindings. Right now we have a mix of manual if->throw and the new
mono_assertTS function, along with some parts where C will assert or C# will throw exceptions, and there isn't a ton of consistency in message formats, etc. It can also be awkward to properly format messages without performance issues.@pavelsavara recently wired up a mechanism in the build tooling so that uses of
mono_assertwill have the exception message inlined, preventing the normal performance overhead, so that's a start. There are still some other criteria we want to satisfy with any final design:I believe the solution is to define a new
errors.tsfile that exports a function for each unique error message, and you invoke the function to throw that error, passing in any relevant values. This puts all the error messages in one place if we decide to localize them later, and it makes it easy to assign them unique identifiers at a glance. The messages having unique identifiers also means that you can search the whole codebase for a given error using that ID instead of merely hoping that every scenario used the same error text.I haven't had a chance to prototype it yet, but it would look something like this:
and then a call site would look like:
This approach would also allow us to customize error handling on a per-error basis (for example, updating a specific error to write a log message before throwing).