Should exceptions not be thrown in Node.js?

Question

I just started working on a new project and no one throws exceptions over here. I get that you shouldn't be doing that for asynchronous code, but it seems to be a valid way to do things for the synchronous side of things.

I would typically do something like this:

function(thing, cb) {
  async_funct(thing, function(stuff) {
    let other_stuff = sync_thing(stuff)
    try {
      let more_other_stuff = other_sync_thing(other_stuff)
    } catch(err) {
      cb(err)
    }
    cb(null, err)
  })
}

Where as in the project it would be treated like this:

function(thing, cb) {
  async_funct(thing, function(stuff) {
    let other_stuff = sync_thing(stuff)

    let more_other_stuff = other_sync_thing(other_stuff)
    if(more_other_stuff instanceof Error)
      cb(err)

    cb(null, err)
  })
}

Is this a standard thing? I'm interested to know how others are handling this type of thing and why.

Answer 1

While Neil's answer is pretty accurate, things may change in the future with the advent of async functions in ECMAScript 8. Support for async functions was added in Node 7.6 and Node 8 (LTS). In addition, Node 8 added the util.promisify function which opens up much potential.

Throwing an error inside an async function is the equivalent of rejecting a promise. Instead of having multiple different ways of handling errors (catch for synchronous functions, (err, ...args) for callbacks, and .catch() for promises), asynchronous code will look synchronous and the try/catch construct can be used.

For example:

Using promises, handling async errors with .catch and sync errors with try/catch:

const promisify = require('util').promisify
const fs = require('fs')
const readFileAsync = promisify(fs.readFile)

readFileAsync('foo.txt', {encoding: 'utf8'})
  .then(data => {
    console.log('CONTENT:', data)

    try {
      let moreOtherStuff = syncThing(data)
      return Promise.resolve(otherSyncThing(moreOtherStuff))
    } catch (err) {
      return Promise.reject(err)
    }
  })
  .catch(err => console.error(err))

Using async functions, handling errors with try/catch:

const promisify = require('util').promisify
const fs = require('fs')
const readFileAsync = promisify(fs.readFile)

(async () => {
  try {
    let data = await readFileAsync('foo.txt', {encoding: 'utf8'})
    console.log('CONTENT:', data)
    let moreOtherStuff = syncThing(data)
    return otherSyncThing(moreOtherStuff)
  } catch (err) {
    console.error(err)
    throw new Error(err)
  }
})()

As you can see, the code uses the same try/catch block to handle the error (whether or not this is desirable depends on the use case and personal preference). Regardless, the code looks more synchronous when read top-down.

In addition to all of this, try/catch/finally statements could not be optimized prior to V8 5.3 (the JavaScript engine used in Node 7.x) so performance could be a factor. However this should no longer be the case going forward, as the engine is now supposedly able to optimize those blocks.

Answer 2

This type of thing is not standard, at least not in the projects that I've worked in.

My guess is that the exceptions alter the otherwise normal flow of the code as is. Returning an error is, in a way, like surrounding every call with a try catch. If an error were thrown, then it may potentially bypass code which needs to be run prior to returning an error.

Granted, this is not best practice. You should be throwing and catching exceptions if they are generally exceptions, and the code should be properly prepared to deal with these exceptions wherever a try catch is present in the call stack. Asynchronous functions are a little more tricky to deal with errors, but certainly not impossible.

In your case, there may not be a difference in behavior, though as these things generally go, if you ask a project manager if a major change can be made to a project that could potentially insert bugs that goes against the general accepted norm of the project in question for the scope of no added functionality or benefit other than being structured a little better, the answer is usually no. ;)

Answer 3

The way you say you would typically do it is how I would do it as well, and how most of the node core libraries expect you to do it.

Synchronous code can and will throw exceptions, while async code will either be event emitters (in which case you have to listen to the 'error' event) or have a callback (in which case it's an error-first pattern) or be Promise-based (which have .then() and .catch()).

In all cases except the callback one (sadly), failing to catch an error (either b/c you're not listening for the 'error' event or don't use .catch() to deal with a revoked promise, or dont' use try..catch will result in the application exiting with an error code at runtime.

It is definitely the correct pattern to do something like:

function myFunc(input, callback) {
    let bob = null;
    try{ 
      bob = someSyncFn();
    } catch (e) {
      return callback(e);
    }
    return callback(null, bob);
}

Though, conceptually if you're able to handle the error in the catch statement (for example, if the operation is adding additional data, and you don't really care if it's added or not) then you could swallow that error, though I rarely do in practice. At a minimum, I'll log a warning if I do that.

The other case that's a little different is that in Express, I'll have an error handler that is responsible for reporting sanitized error comments to the client. That error handler (a middleware) will log the real error, res.send() a 'cleaned' error (complete with http status code) to the client, and then swallow the error so that the app doesn't crash.

Answer 4

In JavaScript, you don't have compiler checked exceptions, so there aren't protections in place to make sure callers handle exceptions. For that reason I think it's better practice to return an object that is either an error or a result. That way you force the caller to address the potential for error. This should be properly documented and obvious to the caller. I don't think it's very obvious in your second code snippet because it involves checking the return type. Something like Either from monet.js is, in my opinion, a better solution.

Example (using monet.js):

/**
 * @return {Either[Error,Int]} A random number or an error if the random was zero
 */
function other_sync_thing() {
    const rand = Math.random()
    if (rand === 0) {
        Either.Left(new Error("random crash"))
    }
    return Either.Right(rand)
}

function(thing, cb) {
  async_funct(thing, function(stuff) {
    let other_stuff = sync_thing(stuff)
    // We can either catch and log the error like so:
    other_sync_thing(other_stuff).cata(err => console.err(err), result => cb(result))
    // Or we can bubble the either up to the callback for it to handle like so:
    cb(other_sync_thing(other_stuff))
  }
}