Skip to content

Commit

Permalink
fix(express): change final-handler signature for booleans
Browse files Browse the repository at this point in the history
  • Loading branch information
jeremyben committed Jan 13, 2022
1 parent 4649862 commit e796dc1
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 29 deletions.
20 changes: 10 additions & 10 deletions express/README.MD
Original file line number Diff line number Diff line change
Expand Up @@ -990,7 +990,7 @@ app.use(finalHandler({
sendAsJson: 'from-response-type',
log: '5xx',
exposeMessage: '4xx',
exposeName: 'always',
exposeName: '4xx',
cleanStatusAndHeaders: true,
notFoundHandler: true
}))
Expand All @@ -1000,9 +1000,9 @@ app.use(finalHandler({

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: 'always'` always sends the error with `res.json`.
* `sendAsJson: true` always sends the error with `res.json`.

* `sendAsJson: 'never'` sends the error with `res.send` (default).
* `sendAsJson: false` sends the error with `res.send` (default).

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

Expand All @@ -1025,8 +1025,8 @@ Express default error handler always sends a `text/html` response ([source code]

##### `log`

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

##### `logger`
Expand All @@ -1048,11 +1048,11 @@ finalHandler({

Error `message` and `name` are not serialized by default. These options make `message` or `name` enumerable so they can be serialized.

* `'always'` always reveals the property (beware of information leakage).
* `'never'` never reveals the property (default).
* `true` always reveals the property (beware of information leakage).
* `false` never reveals the property (default).
* `'4xx'` only reveals the property on client errors.

_`'5xx'` is not available as an option, to avoid information leakage to the client. In that regard, beware of the `'always'` option as well._
_`'5xx'` is not available as an option, to avoid information leakage to the client. In that regard, beware of passing `true` as well._

##### `cleanStatusAndHeaders`

Expand Down Expand Up @@ -1095,8 +1095,8 @@ import { UserRouter } from './user.router'
@Send({ json: true })
@Use(express.json(), express.urlencoded())
@Catch(finalHandler({
sendAsJson: 'always',
log: 'always',
sendAsJson: true,
log: true,
notFoundHandler: true,
}))
class MyApp extends Application {
Expand Down
4 changes: 2 additions & 2 deletions express/__tests__/final-handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ describe('final handler', () => {
@Catch(
finalHandler({
sendAsJson: 'from-response-type',
log: 'always',
log: true,
exposeMessage: '4xx',
exposeName: 'always',
exposeName: true,
cleanStatusAndHeaders: true,
notFoundHandler: true,
})
Expand Down
34 changes: 17 additions & 17 deletions express/src/final-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import * as express from 'express'
* @example
* ```ts
* app.use(finalHandler({
* sendAsJson: 'always',
* sendAsJson: true,
* log: '5xx',
* revealErrorMessage: true,
* revealErrorName: true,
Expand Down Expand Up @@ -56,13 +56,13 @@ export function finalHandler(options: finalHandler.Options): express.ErrorReques

if (err instanceof Error) {
const revealMessage =
options.exposeMessage === 'always' || (options.exposeMessage === '4xx' && res.statusCode < 500)
options.exposeMessage === true || (options.exposeMessage === '4xx' && res.statusCode < 500)

if (revealMessage) {
Object.defineProperty(err, 'message', { enumerable: true })
}

const revealName = options.exposeName === 'always' || (options.exposeName === '4xx' && res.statusCode < 500)
const revealName = options.exposeName === true || (options.exposeName === '4xx' && res.statusCode < 500)

if (revealName) {
// Doesn't work without reassigning value.
Expand All @@ -75,16 +75,16 @@ export function finalHandler(options: finalHandler.Options): express.ErrorReques
if (options.log) {
const logger = options.logger || ((errr) => setImmediate(() => console.error(errr)))

if (options.log === 'always' || (options.log === '5xx' && res.statusCode >= 500)) {
if (options.log === true || (options.log === '5xx' && res.statusCode >= 500)) {
logger(err)
}

// no need to handle 'never'
// no need to handle false
}

// ─── Json ───

if (options.sendAsJson === 'always') {
if (options.sendAsJson === true) {
return res.json(err)
}

Expand All @@ -108,7 +108,7 @@ export function finalHandler(options: finalHandler.Options): express.ErrorReques
}
}

// no need to handle 'never'
// no need to handle false

next(err)
}
Expand All @@ -130,20 +130,20 @@ export namespace finalHandler {
export interface Options {
/**
* Specifies behavior to use `res.json` to send errors:
* - `'always'`: always send as json.
* - `'never'`: pass the error to `next`.
* - `true`: always send as json.
* - `false`: pass the error to `next`.
* - `'from-response-type'`: looks for a json compatible `Content-Type` on the response (or else pass to `next`)
* - `'from-response-type-or-request'`: if the response doesn't have a `Content-type` header, it looks for `X-Requested-With` or `Accept` headers on the request (or else pass to `next`)
*/
sendAsJson: 'always' | 'never' | 'from-response-type' | 'from-response-type-or-request'
sendAsJson: boolean | 'from-response-type' | 'from-response-type-or-request'

/**
* log error:
* - `'always'`: every error
* - `'never'`: none
* - `true`: every error
* - `false`: none
* - `'5xx'`: only server errors
*/
log?: 'always' | 'never' | '5xx'
log?: boolean | '5xx'

/**
* Custom logger
Expand All @@ -155,17 +155,17 @@ export namespace finalHandler {
* Make error `message` enumerable so it can be serialized.
*
* @remarks
* Beware of information leakage with the `'always'` option.
* Beware of information leakage if you pass `true`.
*/
exposeMessage?: 'always' | 'never' | '4xx'
exposeMessage?: boolean | '4xx'

/**
* Make error `name` enumerable so it can be serialized.
*
* @remarks
* Beware of information leakage with the `'always'` option.
* Beware of information leakage if you pass `true`.
*/
exposeName?: 'always' | 'never' | '4xx'
exposeName?: boolean | '4xx'

/**
* Remove `status`, `statusCode` or `headers` properties from error object, once they are applied to the response.
Expand Down

0 comments on commit e796dc1

Please sign in to comment.