Errors passed to next are not mapped

[tpluscode]

Current

Express has a built-in way for returning errors from any middleware with the next(error) call. It appears that errors returned this way do not get converted into problem+json because the handler is an ordinary middleware callback.

Desired

Any error returned in the standard way should be intercepted and converted into a mapped problem document.

Proposal

I think that following best practices of express, the 4-parameter error callback could be used instead. Thus the code can actually be simplified. Here's my implementation:

import { IMappingStrategy } from 'http-problem-details-mapper'

class HttpProblemResponseOptions {
  public strategy: IMappingStrategy;
}

export function HttpProblemResponse (options: HttpProblemResponseOptions) {
  const { strategy } = options

  return (err, req, res, next) => {
    let problem = strategy.map(err)

    res.statusCode = problem.status
    res.setHeader('Content-Type', 'application/problem+json')
    res.send(JSON.stringify(problem))
  }
}

The only difference is that it must be the last app.use call when setting up express

const server = express()
-server.use(HttpProblemResponse({strategy}))

server.get('/', async (req: express.Request, res: express.Response): Promise<any> => {
  return res.send(new NotFoundError({type: 'customer', id: '123' }))
})
+server.use(HttpProblemResponse({strategy}))

[AlexZeitler]

Makes sense. Would you mind sending a PR?

[tpluscode]

Definitely. This will be a breaking change the if implemented as above. Is that fine, or would you want to keep compatibility?

[AlexZeitler]

The reason why we didn't choose the 4-parameter error callback is that we've often seen loggers being registered using last app.use call to handle global exceptions.

I think we should handle this in some way.

[tpluscode]

Ok, I re-read your comment and I think I was right earlier. This seems in line with the express docs. The global logger must simply pass on the error and the HttpProblemResponse comes last:

server.use(function logErrors (err, req, res, next) {
  console.error(err.stack)
  next(err)
})
server.use(HttpProblemResponse({strategy}))

[AlexZeitler]

Ok, I think we should use the 4-parameter error callback and add your sample to the docs.

[tpluscode]

About the docs. I notice that they instruct to implement a IMappingStrategy but there is actually such a default implementation so seems redundant?

[AlexZeitler]

Yes, you're right. I guess it's a copy/paste failure from the http-problem-details-mapper repo.