Wasm bindings need an error handling overhaul

[kg]

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:

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:

    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).

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:

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:

    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]

I prefer mono_assert form in source code, because it explains it's unexpected in normal flow.
It's also only one line, so it doesn't distract my reading of normal-flow branching of the method below.

    const asm = cwraps.mono_wasm_assembly_load(assembly);
    mono_assert(asm, () => errors.assembly_not_found(assembly));

That will need one more inlining regexp.