9

In my Node.js app I added the following code to catch every uncaught exception:

process.on('uncaughtException', function (err: Error) {
    try {
        logger.err(err);
    } catch (err) {

    }
});

The problem is that Express has its own default error handler, which catches every uncaught exception. Now, Express caught the exceptions before Node (process.on), so my logger didn't get reached. However, it's possible to add another error handler that can catch every exception before Express does:

app.use(logErrors);

function logErrors (err: Error, req: Request, res: Response, next: NextFunction) {
    logger.err(err);
    next(err);
}

This still doesn't cover every case. Whenever I have an async function that I call with await, there's no exception, but a rejected Promise is returned instead. For example:

app.get('/foo', async function (req: Request, res: Response, next: NextFunction) {
    await bar();
});

function bar() {
    throw new Exception();
}

won't reach my logErrors function because it won't throw, but will return a rejected Promise.

So to fix it, I wrapped my Express HTTP handlers with another function:

app.get('/foo', wrap(async function (req: Request, res: Response, next: NextFunction) {
    await bar();
}));

function wrap(func: (req: Request, res: Response, next: NextFunction) => void) {
    return async function (req: Request, res: Response, next: NextFunction) {
        try {
            await func(req, res, next);
        }
        catch (err) {
            next(err);
        }
    }
}

next(err) passes the error to my handler. Now, I manage to catch the exceptions to my logErrors function.

I'm almost done. I still have one case in which I can't catch the errors. It happens when I call an async function without the await keyword (It's sometimes useful to use the same function in two different places in the code, once calling in asynchronously, and once synchronously). So this code won't catch the error:

app.get('/foo', wrap(async function (req: Request, res: Response, next: NextFunction) {
    bar();
}));

What's happening here is that the Express HTTP handler returns a resolved Promise to the wrap function. The wrap function in turn, does not reach the catch block, so it does not call to next(err) which would reach my logger.

bar function in turn, returns a rejected Promise, but there is no one waiting for its return value.

How can I change my code in such a way I won't end up with any unhandled Promise rejection? (only generic solution)

Alon
  • 10,381
  • 23
  • 88
  • 152
  • 1
    I think it would be a bad idea and actually impossible to call async function synchronously. You can ignore the fact that the function is async and call it, but it will still run asynchronously, you will just get the problems you describe. So my suggestion is not to use async functions in the sync manner - they are completely different beasts. – Amid Jan 10 '17 at 13:42
  • @Amid they are not really sync. What I meant is: when I call it without 'await' is asynchronous. When I call it with 'await' is synchronous (not really, but kind of: The code is executed line by line, but the thread is available to handle other requests every time I call a promise under the hood). – Alon Jan 10 '17 at 13:46
  • That is what I mean. By calling async function without await you get this kind of problems. They are not actually supposed to be called this way, and your example with exceptions that are really not exceptions but promise rejections is a good illustration why. So I would not recommend doing so even in case if they are the last line of some minor event handler or such - it would still be a good practice to call them with await. – Amid Jan 10 '17 at 13:49
  • @Amid you're right, I agree with you and I always call async functions with 'await' even if it's the last line of a task. I did it that way because I wanted to return a response to the client without making him wait. How else can I do that? I've heard that it's not a very good idea to call to res.end() because it will skip middlewares that are supposed to be executed. – Alon Jan 10 '17 at 14:04
  • I think you have to choose here. Either you return execution to client and it will not get any information about errors/exceptions. Or you wait for async operation to complete and have all info available. – Amid Jan 10 '17 at 14:07
  • @Amid I think I've thought of a solution. Look at the answer I've added. Here instead of calling the function with "await", I call it with "dontAwait". So I don't have to compromise and choose between error handling and being user-freidnly :-) – Alon Jan 10 '17 at 14:44
  • @Alon Didn't you know about another `process.on` event you can set up a listener for - [unhandledRejection](https://nodejs.org/api/process.html#process_event_unhandledrejection) – Kirill Rogovoy Jan 10 '17 at 14:47
  • @KirillRogovoy no, actually I didn't. I just tried it and it's awesome! Please write it as an answer and I will accept it. – Alon Jan 10 '17 at 14:51
  • @Alon Done. Have a good code! – Kirill Rogovoy Jan 10 '17 at 14:55
  • Thanks for this question. A simple `process.on('uncaughtException', (err) => console.error(err.stack));`, placed before `app.listen(PORT)` worked great for me (while checking NODE_ENV etc for production vs development). – user3773048 Jan 25 '19 at 00:50

3 Answers3

2

There is another process.on event you can set up a listener for - unhandledRejection.

You can use it to handle those rejections all over the code.

NOTE: remember to terminate your process after you logged everything you needed. More on this here.

Kirill Rogovoy
  • 583
  • 3
  • 11
1

I've found a solution:

app.get('/foo', async function (req: Request, res: Response, next: NextFunction) {
    dontAwait(() => bar());
});

async function dontAwait(func: () => void) {
   try {
       await func();
   }
   catch (err) {
     logErrors(err);
   }
}
Alon
  • 10,381
  • 23
  • 88
  • 152
1

I still have one case in which I can't catch the errors.

You could, but you currently simply don't do anything with the result. If you want your usual error handlers to trigger, you need to await it.

It happens when I call an async function without the await keyword

I can't see any good reason for that. But if you really want to fire and forget the function, you can do it. You just have to handle errors explicitly:

bar().catch(e => logger.err(e));
Community
  • 1
  • 1
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Thanks for your response. Your suggestion is better than mine, but I still liked Kirill's one better. BTW there is a good reason why I would want to call an async function without the await keyword. You can see in my correspondence with Amid in the comments below the question. – Alon Jan 10 '17 at 15:09
  • OK, "*I wanted to return a response to the client without making him wait.*" is a reasonable use case, but a rather rare one. However, I don't think it's a case of "fire and forget", since you probably want to do something with the result (success or fail) of the call: store it and notify the user that you're done. `unhandledRejection` tracking is not a good solution for that since it does loose context. Yes, it's a good idea to log them, but you should never *expect* them. – Bergi Jan 10 '17 at 15:16
  • Bergi you're right, unhandledRejection is usually only good for logging. Calling the function without 'await' is useful when the user needs to start a background operation that he does not supposed to get its result in the HTTP response. He can get the result later via email or by comet/polling. During the process, I handle every kind of failure that I could think of. this generic error handling is only supposed to log failures that I didn't expected to. In those cases, I will have to restart my app as Kirill said, and finally I'll have to fix my code to handle those failures. – Alon Jan 10 '17 at 15:39
  • Yes, for that use case go with `unhandledRejection`s. What threw me off was that you wanted to call the default Express error handler, so I thought you'd still wanted some request-specific error handling. – Bergi Jan 10 '17 at 16:04