Missing async/await on ErrorHandlingMiddleware

[Hulle107]

trafficstars

Hej love the package so far.

But have found that you do not really do promise for Error Handling Middleware the same way as you do other middleware. This is what I think could help in that regard.

Suggestion: index.js

383. // Execute error middleware
384. for (const err of this._errors) {
385.   if (response._state === 'done') break;
386.   // Promisify error middleware
387.   await new Promise(async (r) => {
388.     let rtn = await err(e, response._request, response, () => {
389.       r();
390.     });
391.     if (rtn) response.send(rtn); r();
392.   });
393. }
394. // end for

As seen you could implement it the same way as you do for other middleware. IF there is a reason for not do it this way I really wanna know.

[warrickhill]

We are having the same issue. @Hulle107 your code above fixed it for us, have you made a PR for it?

[yidah]

I also had the same error and spent considerable amount of time finding the solution. Thanks for your code @Hulle107. I hope this can be fixed soon.

[warrickhill]

Made a PR with @Hulle107 code

[sean-hernon]

There are actually two issues here -- one is that the asynchronous error handlers were not supported (fixed in PR with async / await addition).

The other is that the promise was only being resolved in the next function. Which leads to some unidiomatic code. This is the problem that I encountered, which I will illustrate here, for anyone that may be experiencing the same problem.

It's normal to be able to do the following in an error handling middleware -

(err, req, res, next) => {
    if (condition1) {
        return res.status(400).send('BAD REQUEST');
    }

    if (condition2) {
        return res.status(403).send('FORBIDDEN');
    }

    if (condition3) {
        return res.status(404).send('NOT FOUND');
    }

    //etc

    next();
}

In the above example, you return early when you send the response, instead of calling the next function, which is the usual flow for a middleware, but following this pattern the promise in index.js never resolves, without the addition of the final r();.

The workaround for anyone experiencing this, is that you cannot return early and must currently call the next function (pending the above PR being merged).