Blog of Rob Galanakis (@robgalanakis)

Failed assertions and async functions

On 26 Feb, 2018 By With 0 Comments

Armin Ronacher asked in a tweet:

I explored this quite a bit working in JavaScript on the client and feel like I have a good understanding of the tradeoffs (tl;dr is to return a rejected promise with an Error, but I explain how we made this work well).

First off, I’m usually very strict about erroring/raising/throwing when a programmer error happens, like passing in an invalid value. I want programmer errors to raise loudly, as early as possible, so they can be fixed. This is originally the approach I went with at Cozy.

However, most of the event handlers that called async code had a form like:

turnOnLoadingState();
doAsyncThing(args).
  tap(showSuccess).
  tapCatch(showError). // See http://bluebirdjs.com/docs/api/tapcatch.html
  finally(turnOffLoadingState);

If doAsyncThing raised an error (asserted), turnOffLoadingState would never be called, and the loading state would continue to be active.

The perpetual loading state is to me the most unacceptable user state. I would much rather have a user rage-clicking a button than I would show them an inactive screen or button that is ‘still processing.’ An obviously broken button is obvious; the user can bug out or reload or whatever once they give up. A stuck loading state begs the user to hang on until they get frustrated and leave, unsure if what they did ever worked, and what the state of the world will be when they try again.

Failed assertions were triggered too often, usually due to the chaos, asynchrony, and unpredictable behavior of browser JavaScript and the DOM. The story may be different using certain frameworks or under certain conditions, but in real world browser code, I suspect not (hello browser extensions!).

To avoid a perpetual loading state in the face of a non-negligible number of failed assertions, doAsyncThing should return an Error wrapped in a rejected promise:

function doAsyncThing(id) {
  if (typeof id !== 'number' || number < 1)
    return Promise.reject(new Error(`id must be a positive number, got %{id}`));
  // ...
}

This way, in the event handler code above, showError and turnOffLoadingState are both called even with an invalid argument. showError won’t show quite the right error- it’ll probably be something generic like “Something went wrong, please try again”- but it’ll be good enough.

The key to making this work well- that is, to still have a strict tolerance for programmer errors- is setting up an unhandled promise rejection handler, which you can do in most mainstream promise libraries. When a promise is rejected with an Error, we can log an alert to Sentry or wherever else, instead of raising an error and stopping the program dead. In Bluebird, that’d look something like this:

// See http://bluebirdjs.com/docs/api/error-management-configuration.html#global-rejection-events
window.addEventListener("unhandledrejection", function (e) {
  e.preventDefault();
  if (e.detail.reason instanceof Error) {
    Raven.captureException(e.detail.reason);
  }
});

This gave us a best-of-all-cases scenario for our use case (no perpetual loading state, but report programmer errors). That said, I’d want to offer a few caveats, some of which I’ve already gone over:

  • What I’d much prefer is a strong system of contracts to ensure a function can’t be called with an invalid argument. This is true for me in all programming languages, but I wanted to reiterate it here. There are options for this on the frontend now, but nothing that was practical at Cozy.
  • Again, I initially insisted we do actually throw an error from failed assertions in async methods, because I could look at most code and say “it’s not possible this code is called if the application is in an invalid state.” But I was constantly wrong! (Shoutout to Piet van Zoen who was correct from the start) You could be pedantic and say these were all “bugs,” and technically they were, but they were also often irreproducible and harmless (usually API GET requests).
  • If I were building a library for 3rd party distribution, rather than 1st party application code, I would be more strict about how my assertions fail, and throw an error. It is paramount to me that we learn about failed assertions: if I can’t guarantee a global promise rejection handler is installed (I cannot when writing a 3rd party library), I’d rather throw.

Leave a Reply

«
»