Error handling

[b-ma]

Currently, calls to web-audio-api-rs are done without worrying for panics that may occur, which is probably not a good idea. Maybe wrap all calls to the parent lib in https://doc.rust-lang.org/std/panic/fn.catch_unwind.html to have proper Error handling.

[b-ma]

for error types (i.e. IndexSizeError) it seems there is no way to handle that on the rust side. Maybe monkey patch all methods on the JS side and use something like https://www.npmjs.com/package/custom-error-instance would work...

[orottier]

Alternatively, we should introduce fallible methods (e.g. AudioContext::try_new(opts) -> Result<AudioContext, ErrType>) for all sites that can panic.
This is a lot of work though, but it will be beneficial for other uses cases as well. I think it is preferable to wrapping in catch_unwind
Another drawback is that is pollutes the API (double the number of methods, worst case). We could hide it behind a feature flag?
Maybe first we should make a list of all fallible methods

[b-ma]

Hum, I understand and maybe that could be something important to do, I really don't realize how rust users behave on such concerns :). But in any case I don't think this would help much on the JS side, this is really something related to the bindings as it seems Napi can only trigger Error, TypeError and RangeError (cf. https://nodejs.org/api/n-api.html#exceptions).

I've ended up trying something like this for now on the rust side, which seems to work quite well and is far more simple and less noisy than wrapping all calls into catch_unwind:

std::panic::set_hook(Box::new(|panic_info| {
    println!(
        "panic catched: {}",
        panic_message::panic_info_message(panic_info)
    );
}));

Then as the panic messages from Rust are quite well formatted and consistent, I think I could re-trigger the right error type just by parsing the message on the JS side (a bit some kind of brute force hacky monkey-patch but I don't really see how to do that another way...)

Actually, I don't personally consider much important (at least in terms of usability) that the error is an instance of Error or IndexSizeError as long as the error message is clear... but these are checked in the wpt so it's lot of noise I'd like to suppress at some point so we can see more clearly the real problems (which from what I have seen so far are mostly due to the bindings...)

[b-ma]

Could work with Proxy:

function monkeyPatchGainNode(nativeBinding) {
  class GainNode extends nativeBinding.GainNode {
    constructor(...args) {
      super(...args);

      return new Proxy(this, {
        get(obj, prop) {
           // ...
        },
        set(obj, prop, value) {
          try {
            return Reflect.set(obj, prop, value);
          } catch (err) {
            if (err.message.startsWith('NotSupportedError')) {
              throw new NotSupportedError(err.message.replace(/^NotSupportedError - /, ''));
            }
            // etc...
          }

          return true;
        },
        apply(target, thisArg, argumentsList) {
           // ...
        },
      });
    }
  }

  return GainNode;
}

[b-ma]

• Most of the heavy / architectural work done in Feat - error handling #66
• Some follow up in Review AudioBuffer #67
• We can also rely on wpt now to improve things on the fly