Why is `.catch(err => console.error(err))` discouraged?

Benjamin Gruenbaum picture Benjamin Gruenbaum · Jun 17, 2018 · Viewed 12.2k times · Source

I'm using promises and have code that looks like the following:

function getStuff() { 
  return fetchStuff().then(stuff => 
    process(stuff)
  ).catch(err => {
    console.error(err);
  });
}

Or:

async function getStuff() { 
  try {
    const stuff = await fetchStuff();
    return process(stuff);
  } catch (err) { 
    console.error(err);
  }
}

I was doing this to avoid missing on errors but was told by a fellow user that I shouldn't be doing this and it is frowned upon.

  • What's wrong with return ….catch(err => console.error(err))?
  • I've seen a lot of code that does this, why?
  • What should I do instead?

Answer

Benjamin Gruenbaum picture Benjamin Gruenbaum · Jun 17, 2018

Why does old code do this?

Historically, older (pre 2013) promise libraries 'swallowed' unhandled promise rejections you have not handled yourself. This has not been the case in anything written since then.

What happens today?

Browsers and Node.js already automatically log uncaught promise rejections or have behaviour for handling them and will log them automatically.

Moreover - by adding the .catch you are signalling to the method calling the function that undefined is returned:

// undefined if there was an error
getStuff().then(stuff => console.log(stuff)); 

The question one should be asking oneself when writing async code is usually "what would the synchronous version of the code do?":

function calculate() { 
  try {
    const stuff = generateStuff();
    return process(stuff);
  } catch (err) { 
    console.error(err);
    // now it's clear that this function is 'swallowing' the error.
  }
}

I don't think a consumer would expect this function to return undefined if an error occurs.

So to sum things up - it is frowned upon because it surprises developers in the application flow and browsers log uncaught promise errors anyway today.

What to do instead:

Nothing. That's the beauty of it - if you wrote:

async function getStuff() { 
  const stuff = await fetchStuff();
  return process(stuff);
}
// or without async/await
const getStuff = fetchStuff().then(process);

In the first place you would get better errors all around anyway :)

What to do if I'm running an old version of Node.js?

Old versions of Node.js might not log errors or show a deprecation warning. In those versions you can use console.error (or proper logging instrumentation) globally:

// or throw to stop on errors
process.on('unhandledRejection', e => console.error(e));