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.

stimulus/src/core/binding.ts

Lines 74 to 84 in 5fd12c7

	private invokeWithEvent(event: ActionEvent) {
	const { target, currentTarget } = event
	try {
	this.method.call(this.controller, event)
	this.context.logDebugActivity(this.methodName, { event, target, currentTarget, action: this.methodName })
	} catch (error: any) {
	const { identifier, controller, element, index } = this
	const detail = { identifier, controller, element, index, event }
	this.context.handleError(error, `invoking action "${this.action}"`, detail)
	}
	}

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