Skip to content

Instantly share code, notes, and snippets.

@joeytwiddle
Last active February 17, 2023 05:57
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save joeytwiddle/21331531f50885345e063d59ec7f11f2 to your computer and use it in GitHub Desktop.
Save joeytwiddle/21331531f50885345e063d59ec7f11f2 to your computer and use it in GitHub Desktop.
Using throw instead of Promise.reject()

I recommend using throw over return Promise.reject() when possible, because:

  1. If you are inside "promise-land" then your throw will be automatically converted into a rejected promise.
  2. But it is shorter, which makes it easier to read.
  3. And it retains some parity between synchronous and asynchronous code. (It will continue to work as expected if those lines were extracted into a synchronous function.)
  4. Also, throw can be used to break out of a stack of deeply nested synchronous function calls. (Returning a rejection will only pass it up one level. Of course, we shouldn't be returning rejections from sycnhronous functions anyway.)

Although throwing an error is not safe inside callback functions (which are often called from the engine, without a surrounding try-catch wrapper), it is entirely safe to throw from inside an async function (or from within a .then() handler function), because in those cases the throw will be automatically converted into a rejected promise.

Situations where it is safe to use throw

Whenever we are inside an async function or a .then() handler function. We can call this "promise-land".

async function foo(bar) {
  if (typeof bar !== 'number') throw Error('bar must be a number');
}
getUser(userId).then(user => {
  if (!user) throw Error('User was not found');
  // do something with user
}).catch(console.error);

When is it not safe to use throw?

When we are in synchronous code, an error will usually cause the Node process to crash. Node does this to ensure the app does not continue to run in an invalid/inconsistent state.

Inside a callback function:

fs.readFile('./config.txt', (err, data) => {
  // WARNING!  This will likely crash the process.
  if (err) throw err;
  // do something with data
});

If you don't want the process to crash, then you had better handle this error some other way. Some alternatives are:

// Pass the error to an outer callback:
if (err) { callback(err); return; }
// or just log it and abort:
if (err) { console.error(err); return; }

You should only ignore errors if you are confident that the error has not compromised the integrity of your app's data.

Inside a callback function which is inside promise scope:

getUser(userId).then(user => {
  // This error will be converted into a rejected promise
  if (!user) throw Error('User was not found');

  fs.readFile('./config.txt', (err, data) => {
    // But this error will not be converted, because it this code is not running
    // inside the promise code.  (It's an entirely different, later stack.)
    // BAD!
    if (err) throw err;
    // do something with data
  });
}).catch(console.error);

In this case a simple solution would be to promisify fs.readFile so we can stay in promise-land.

const readFilePromisified = utils.promisify(fs.readFile);

getUser(userId).then(async user => {
  // This error will be converted into a rejected promise
  if (!user) throw Error('User was not found');

  // We don't even need to think about handling the error here.
  // It will bubble up automatically.
  const data = await readFilePromisified('./config.txt');

  // do something with data
}).catch(console.error);

Inside a Promise creating function

This is perhaps the most surprising case, so look out for this one.

function getUser(userId) {
  if (typeof userId !== 'string') throw Error('userId must be a string');
  // Perform the user lookup
}

getUser(12).then(user => {
  console.log("Got user: " + user);
}).catch(error => {
  console.error("Caught the error: " + error);
});

When we called getUser we passed a number instead of a string, so it throws a validation error. But this error will not be caught by our promise error handler. Why? Because the error was actually thrown in synchronous code, not in promise-land.

There are three possible ways to solve this:

  1. Make getUser an async function. Then the throw will happen in promise-land, and be converted into a rejection. This is an easy solution if you are using ES7.
  2. Instead of throw-ing, return Promise.reject(...) when the userId has the wrong type.
  3. At the call site, be cautious. I will show this one:
Promise.resolve().then(() => {
  return getUser(12);
}).then(user => {
  console.log("Got user: " + user);
}).catch(error => {
  console.error("Caught the error: " + error);
});

This is quite a handy solution if, as a caller, you do not trust the function that you are calling to always return a promise. Any error thrown synchronously inside getUser() will be become a rejected promise, and will be passed to our handler.

  1. Another similar solution can be formed using an IIAFE:
(async () => {
  const user = await getUser(12);
  console.log("Got user: " + user);
})().catch(error => {
  console.error("Caught the error: " + error);
});

That also gets us into promise land before calling getUser.

  1. Use a try-catch which treats synchronous errors the same as async errors.
(async () => {
  try {
    const user = await getUser(12);
    console.log("Got user: " + user);
  } catch (error) {
    console.error("Caught the error: " + error);
  }
})();

Situations where it is not possible to use throw

Sometimes a developer wants to do everything in one expression:

getUser(userId).then(user => {
  // INVALID SYNTAX
  return user ? increaseKarma(user, 10) : throw Error('User was not found');
}).catch(console.error);

In fact it is possible to throw from within an expression, but it requires a helper function:

getUser(userId).then(user => {
  return user ? increaseKarma(user, 10) : throwError(Error('User was not found'));
}).catch(console.error);

function throwError(error) {
  throw error;
}

Some developers might like to call that function die(), to mirror features in other languages such as Perl.

It is debatable whether hiding flow constructs inside an expression adds clarity or detracts from it.

In the case above, since it is clear that we are returning from an async (promise-returning) function, then returning a rejection would be suitable:

getUser(userId).then(user => {
  return user ? increaseKarma(user, 10) : Promise.reject(Error('User was not found'));
}).catch(console.error);

If we look at a different example, where we are not immediately returning:

getUser(userId).then(user => {
  const usersKarma = user ? user.karma : throwFromExpression(Error('User was not found'));
  console.log("Karma:", usersKarma);
}).catch(console.error);

Then it is probably clearer to separate the guard logic from the property extraction:

getUser(userId).then(user => {
  if (!user) throw Error('User was not found');
  const usersKarma = user.karma;
  console.log("Karma:", usersKarma);
});

but if we had to make many extractions from different objects, then a case could be made for the more compact form.

@joeytwiddle
Copy link
Author

This recommendation was adopted into Loopback 4's styleguide! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment