Exceptions in async controller action not caught by stimulus

[sh-jthorpe]

Stimulus doesn't catch exceptions thrown from async controller actions. This means that

a) We don't get nice stimulus errors in the console b) handleError is never called, so overriding it (e.g. to log errors to a monitoring system like Sentry or Azure App Insights) has no effect.

This happens because the call to user code (this.method.call(this.controller, actionEvent)) is not awaited, so it returns a Promise immediately, and escapes the try catch block.

https://github.com/hotwired/stimulus/blob/5fd12c7b6af88620741e3aa969b6ab440559c5b7/src/core/binding.ts#L74-L84

I think something like this would work:

  invokeWithEvent(event) {
    function handleError(error) {
      const { identifier, controller, element, index } = this;
      const detail = { identifier, controller, element, index, event };
      this.context.handleError(error, `invoking action "${this.action}"`, detail);
    }

    const { target, currentTarget } = event;
    try {
      const { params } = this.action;
      const actionEvent = Object.assign(event, { params });
      Promise.resolve(this.method.call(this.controller, actionEvent)).catch(error => handleError(error));
      this.context.logDebugActivity(this.methodName, { event, target, currentTarget, action: this.methodName });
    } catch (error) {
      handleError(error);
    }
  }

I think this is really important, because without the ability to configure custom error handling that applies to all stimulus controller code, users running production apps wont have visibility into how their async actions are failing, without putting a try catch block into every one of those methods.

[sh-jthorpe]

Hi, just wondering if anyone is monitoring this repo?

I'm happy to submit a PR if I need to, I'd just like to know that I'm on the right track.

[adrienpoly]

@sh-jthorpe a PR with an attached test case would be nice indeed