Skip to content

Commit

Permalink
fix(express): easier and more flexible finalHandler
Browse files Browse the repository at this point in the history
  • Loading branch information
jeremyben committed Jun 1, 2022
1 parent ea4964e commit 32fb80f
Show file tree
Hide file tree
Showing 4 changed files with 158 additions and 93 deletions.
67 changes: 38 additions & 29 deletions express/README.MD
Original file line number Diff line number Diff line change
Expand Up @@ -1004,29 +1004,29 @@ const app = express()
register(app, [ThingRouter])

app.use(finalHandler({
sendAsJson: 'from-response-type',
json: 'from-response-type',
log: '5xx',
notFoundHandler: true
}))
```

##### `sendAsJson`
##### `json`

Express default error handler always sends a `text/html` response ([source code](https://github.com/pillarjs/finalhandler/blob/v1.1.2/index.js#L272-L311)). This doesn't go well with today's world of JSON APIs.

* `sendAsJson: true` always sends the error with `res.json`.
* `json: true` always sends the error with `res.json`.

* `sendAsJson: false` passes the error to `next` to be handled by express final handler (default).
* `json: false` passes the error to `next` to be handled by express final handler (default).

* `sendAsJson: 'from-response-type'` sends the error with `res.json` by looking for `Content-Type` on the response:
* `json: 'from-response-type'` sends the error with `res.json` by looking for `Content-Type` on the response:

```ts
res.type('json')
// ...
throw Error('Nope') // Content-Type: 'application/json'
```

* `sendAsJson: 'from-response-type-or-request'` first looks for `Content-Type` on the response, or infers it from `X-Requested-With` or `Accept` headers on the request:
* `json: 'from-response-type-or-request'` first looks for `Content-Type` on the response, or infers it from `X-Requested-With` or `Accept` headers on the request:

```http
GET http://host/foo
Expand All @@ -1037,52 +1037,61 @@ Express default error handler always sends a `text/html` response ([source code]
throw Error('Nope') // Content-Type: 'application/json'
```

##### `exposeInJson`
##### `expose`

By default, Error `message` and `name` are not serialized to json.

With this option, you can either hide all error properties or expose some of them in the json response:
With this option, you can either hide all error properties or expose some of them in the serialized response:

- `true`: exposes all properties (stack included, beware of information leakage !), _default in development_.
- `false`: exposes nothing (empty object), _default in production `NODE_ENV === 'production'`_.
- array of strings: whitelists specifics properties.

You can pass a function with the status code as parameter for more conditional whitelisting:
* `true`: exposes all properties (stack included, beware of information leakage !).
* `false`: exposes nothing (empty object).
* `string[]`: whitelists specifics properties.
* `(status) => boolean | string[]`: function for more conditional whitelisting:

```ts
finalHandler({
sendAsJson: true,
exposeInJson(statusCode) {
// expose all properties in development
if (process.env !== 'production') return true
json: true,
expose(status) {
// expose all properties in non production environment
if (process.env !== 'production') {
return true
}
// expose only some properties of client errors in production
return statusCode < 500 ? ['message', 'data'] : false
if (status < 500) {
return ['message', 'code', 'data']
} else {
return false
}
}
})
```

##### `log`

* `log: true` always logs errors.
* `log: false` never logs errors (default).
* `log: '5xx'` only logs server errors.

##### `logger`
* `log: true` always logs errors (with `console.error`).
* `log: false` never logs errors, _default_.
* `log: '5xx'` only logs server errors (with `console.error`).
* If you need the flexibility to log more infos or use a dedicated logger other that `console.error`, you can pass a function like so:

By default, errors are logged to `stderr` with `console.error`.

You can bind a custom logger like [winston](https://github.com/winstonjs/winston) or [pino](https://github.com/pinojs/pino):
```ts
import * as pino from "pino";
const logger = pino()
finalHandler({
log: '5xx',
logger: logger.error,
log(err, req, res) {
logger.error({
err,
status: res.statusCode,
path: req.url,
timestamp: new Date().toISOString(),
})
},
})
```

_The response object type only exposes safe properties, so you don't send the response by accident._

##### `notFoundHandler`

Like the error handler, Express default route handler always sends a `text/html` response when the route is not found.
Expand Down Expand Up @@ -1122,7 +1131,7 @@ import { UserRouter } from './user.router'
@Use(express.json(), express.urlencoded())
@Router.ScopedMiddlewares
@Catch(finalHandler({
sendAsJson: true,
json: true,
log: true,
notFoundHandler: true,
}))
Expand Down
37 changes: 29 additions & 8 deletions express/__tests__/final-handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,21 @@ afterAll(() => consoleErrorSpy.mockRestore())
describe('final handler', () => {
@Catch(
finalHandler({
sendAsJson: 'from-response-type-or-request',
exposeInJson(statusCode) {
if (statusCode >= 500) return ['name']
json: 'from-response-type-or-request',
expose(status) {
if (status >= 500) return ['name']
return ['name', 'message']
},
log: true,
notFoundHandler: 400,
log(err, req, res) {
setImmediate(() =>
console.error({
err,
status: res.statusCode,
path: req.originalUrl,
})
)
},
notFoundHandler: 499,
})
)
class App extends Application {
Expand Down Expand Up @@ -50,11 +58,18 @@ describe('final handler', () => {
expect(res.header['x-header']).toBe('test')
expect(res.body).toEqual({ name: 'Error', message: 'Impossible' })

expect(consoleErrorSpy).toBeCalledWith(expect.objectContaining({ message: 'Impossible' }))
expect(consoleErrorSpy).toBeCalledWith(
expect.objectContaining({
err: expect.objectContaining({ message: 'Impossible' }),
status: 400,
path: '/foo',
})
)
})

test('html error', async () => {
const res = await rq.get('/bar')

expect(res.status).toBe(422)
expect(res.type).toBe('text/html')
expect(res.text).toContain('UnprocessableEntity: I dont like this data')
Expand All @@ -63,11 +78,17 @@ describe('final handler', () => {
test('default not found error', async () => {
const res = await rq.get('/bar/baz')

expect(res.status).toBe(400)
expect(res.status).toBe(499)
expect(res.type).toBe('text/html')
expect(res.text).toContain('RouteNotFoundError: Cannot GET /bar/baz')
// expect(res.body).toEqual({ name: 'RouteNotFoundError', message: 'Cannot GET /bar/baz' })

expect(consoleErrorSpy).toBeCalledWith(expect.any(Error))
expect(consoleErrorSpy).toBeCalledWith(
expect.objectContaining({
err: expect.any(Error),
status: 499,
path: '/bar/baz',
})
)
})
})
Loading

0 comments on commit 32fb80f

Please sign in to comment.