Why is my error handler in ExpressJS not working?

Question

I am building an api, with ExpressJS, for a project of mine and would like to implement error handling. I've read some articles on the internet on how to do this and tried a few ways of implementing error handling in my api. The problem that I have now is that my error handler doesn't work and I have no idea why. To make my problem more clear, here's what I already tried:

This is the bottom part of my index.js file:

// Controllers
app.use("/auth", require("./controllers/auth"));
app.use("/clients", require("./controllers/clients"));

// Error handling
app.use((err, req, res, next) => {
    logger.error(
        `date - ${new Date()}, message - ${err.message}, stack trace - ${
            err.stack
        }`
    );

    return res
        .status(500)
        .json({ status: 500, message: "Something went wrong." });
});

const PORT = process.env.PORT || 3001;
app.listen(3001, () => {
    console.log(`Listening on port ${PORT}`);
});

As a side note: I am using WinstonJS for logging errors to files and Mongoose as the ORM for my MongoDB database.

This is the controller for the clients route (controllers/clients.js):

const express = require("express");
const router = express.Router();

const ClientsService = require("../services/clients");

// GET -> Get all clients -> /clients
router.get("/", async (req, res) => {
    const clients = await ClientsService.getAll();

    return res.status(200).json({ status: 200, data: clients });
});

module.exports = router;

And finally, this is the clients service (services/clients.js), the place where I would like to throw errors (maybe this isn't the right place to throw errors, please correct me if I am wrong):

const ClientModel = require("../models/Client");

const getAll = async () => {
    return ClientModel.findById({})
        .then(clients => clients)
        .catch(err => {
            throw new Error(err);
        });
};

module.exports = { getAll };

As you can see, I put an empty object in the function for findById. This is to force an error, so that I can test the error handling. A few lines below that, I throw an error. I expect this to fire the middleware function that I defined in index.js. That middleware function, as shown in the code for index.js, logs the error to a file and then sends a 500 response to the client (read 'user').

Instead of this, I actually get an error in the console called 'UnhandledPromiseRejectionWarning'. Here's the error:

(node:15416) UnhandledPromiseRejectionWarning: Error: CastError: Cast to ObjectId failed for value "{}" at path "_id" for model "clients"
    at D:\projects\myproject\services\clients.js:7:10
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at async D:\projects\myproject\controllers\clients.js:8:18
(node:15416) UnhandledPromiseRejectionWarning: Unhandled promise rejection. 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(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:15416) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise 
rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Why is my error handling not working? If you have a solution, how would I be able to enable error handling for my code?

Thank you in advance!

score 10 · Accepted Answer · answered Mar 21 '20 at 13:03

In the express error handling documentation, it says:

You must catch errors that occur in asynchronous code invoked by route handlers or middleware and pass them to Express for processing.

So you need to catch the error, and pass it to the express error middleware using next(err):

So your controller needs to be updated like this:

router.get("/", async (req, res, next) => {
  try {
    const clients = await ClientsService.getAll();
    return res.status(200).json({ status: 200, data: clients });
  } catch (err) {
    next(err);
  }
});

Also you had better to return a promise from your service like this which is clearer:

const getAll = () => {
  return ClientModel.findById({});
};

Now if you are concerned to use try catch in every route, you may check this answer to avoid it.

I tried to mock it by throwing an error outside of the try/catch block or throwing an error inside the catch block. They could not be handled by the error handler. Is there a way to handle the "unexpected" errors? — LeonF, Sep 03 '21 at 16:50

score 1 · Answer 2 · answered Mar 21 '20 at 12:10

1

Make the try/catch before calling the async function

try{
    const clients = await ClientsService.getAll();
    return res.status(200).json({ status: 200, data: clients });
}catch(err){
    return res.status(500).json({ status: 500, message: 'There was a problem :' + err });
}

answered Mar 21 '20 at 12:10

Nathan Lo Sabe

197
1
13

Does that mean that I will have to wrap every controller in a try/catch statement? – Jonathan van de Groep Mar 21 '20 at 12:30
You could throw the error inside the promise or reject the promise instead. – Nathan Lo Sabe Mar 21 '20 at 12:51
In the [doc](https://developer.mozilla.org/es/docs/Web/JavaScript/Referencia/Objetos_globales/Promise/catch) you can find useful examples – Nathan Lo Sabe Mar 21 '20 at 12:55

score -1 · Answer 3 · answered Mar 17 '21 at 21:05

-1

If it helps anyone, you should be able to get this down to fairly tidy syntax like this:

this._expressApp.get('/api/somedata', this._catch(this._router.myDataRoute));

Some example code of mine here - see also the blog post from Alex Bazhenov.

answered Mar 17 '21 at 21:05

Gary Archer

22,534
2
12
24

Why is my error handler in ExpressJS not working?

3 Answers3