Why should errors of a Promise be always caught?

Question

I am using @typescript-eslint/no-floating-promises rule in my project. This rule complains if I write code like this -

functionReturningPromise()
    .then(retVal => doSomething(retVal));

It wants me to add a catch block for this Promise. It makes sense to me if I want some logic in my exception handling block. However, in many cases I don't need that. I am fine with the error being thrown. So, to suppress the error fired by this rule I just end up doing this -

functionReturningPromise()
    .then((retVal) => doSomething(retVal))
    .catch((error) => {
        throw error;
    });

Even if I don't add the catch block as above, the error still gets thrown the same way (at least as I see it in my console output). So, I don't get the point of explicitly specifying this catch block. Am I missing something? Is there really a difference in the way the error is thrown if I add/not add the catch block?

The docs say why: _"Floating Promises can cause several issues, such as improperly sequenced operations, ignored Promise rejections, and more."_. If you don't catch the error you can have an unhandled promise rejection, see e.g. https://stackoverflow.com/q/40500490/3001761, which in Node 15 and up is an error: https://nodejs.org/en/blog/release/v15.0.0#throw-on-unhandled-rejections---33021. — jonrsharpe, May 29 '23 at 13:15
Do you also never do any error recovery in sync code? Because the two situations are more or less equivalent - if there is an error *that you can reasonably expect* then best practice is to handle it. Even if the handling involves ignoring the error. Related `.catch((error) => { throw error; });` doesn't do anything useful. The promise will still reject. — VLAZ, May 29 '23 at 13:19
"*I don't need that. I am fine with the error being thrown.*" - if you don't care whether the function works, why are you calling it at all? — Bergi, May 29 '23 at 13:33
`.catch((error) => { throw error; });` is pointless, it's just that the linter rule doesn't recognise the pattern. If you simply want to suppress the warning, you should rather write `// eslint-disable-next-line @typescript-eslint/no-floating-promises` or `void functionReturningPromise().then(doSomething);` where the intent is clear — Bergi, May 29 '23 at 13:36
@VLAZ @Bergi I didn't mean to _never_ have any error recovery. I was talking about a case where I just want the `error` to be thrown without any additional procesing. For example - Let's say a server required some file or some env variable to be set to start up. If it cannot get those things (and let's say I don't have the need for logging, or need a fallback mechanism or anything specific to handle this case) it should just throw the error and crash. — DashwoodIce9, May 29 '23 at 13:51
"*it should just throw the error and crash*" - ok, then you should explicitly ignore the linter warning - and the rule was doing fine, since usually one does not want the application to crash. Also, usually you'd have reading the file or variable be `await`ed and `return`ed, and there would be only a single location - the `main` function - in your application where you would crash if an error occurs during startup. You might even disable the rule altogether for the entrypoint module. — Bergi, May 29 '23 at 13:56

score 2 · Accepted Answer · answered May 30 '23 at 02:17

The commenters have all done a good job of answering your question - but to show why this matters by example, imagine the following code:

Promise.reject();
setTimeout(() => console.log('hello'), 1000);

This code looks pretty innocuous - there is a no-op promise rejection which is unhandled and 1 second after startup the program will log 'hello'.

In a browser - this is exactly what will happen - the browser will log an "uncaught promise rejection" error but it will otherwise ignore it.

However in NodeJS (as of Node v15) unhandled promise rejections are a HARD ERROR - meaning that the process will exit when it detects one!

You can verify this by running the code in your terminal (-e means "evaluate and run this code string")

$ node -e "Promise.reject(); setTimeout(() => console.log('hello'), 1000)"
node:internal/process/promises:288
            triggerUncaughtException(err, true /* fromPromise */);
            ^

[UnhandledPromiseRejection: This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason "undefined".] {
  code: 'ERR_UNHANDLED_REJECTION'
}

Node.js v18.12.1

You can see that the 'hello' never gets printed because the process terminates before 1s!

To validate that things work as expected, you can change the .reject to .resolve:

$ node -e "Promise.resolve(); setTimeout(() => console.log('hello'), 1000)"
hello

So if you're writing a NodeJS application with any LTS supported version you definitely should be handling the errors or else you risk unexpected crashes.

If your code only runs in browsers then you're probably wondering if you should handle the failure - after all your users aren't looking at the console so they won't know anything bad happened so who cares? And you'd be right from a "code only" point of view - however your users want feedback from their application when something goes wrong.

Imagine the following scenario - your promise tracks the result of an API call that submits some data the user entered in your application. If that API call fails for some reason, then your application should respond to that appropriately and tell the user something went wrong.

The alternative of handling it could mean your application shows an infinite loading spinner, or that a user thinks their data was submitted successfully when it actually wasn't. Either way - it's a very bad and broken UX!

Finally doing something like .catch(e => { throw e }) you're not actually handling the error. Sure this code silences the linter - but it all you're doing is creating a new, rejected promise which will get logged to the console. Instead you should wire the error into your application's UI in some way, eg something as simple as .catch(e => { alert(e); throw e }) would even be better.

Thanks Brad. This answers all my questions. Here's my TLDR - 1. For Node 15+, an uncaught promise rejection will crash the application. 2. It's almost always better to `catch` the exception and handle it. 3. `.catch(e => { throw e })` is useless, it does nothing but suppress the linter error. 4. If there's a valid usecase to not `catch` the error, then use - `void functionReturningPromise().then(doSomething);` — DashwoodIce9, May 31 '23 at 13:50

Why should errors of a Promise be always caught?

1 Answers1