Skip to content

Commit

Permalink
Do not send 5xx error messages to the client in production
Browse files Browse the repository at this point in the history
This only applies to the default error handler.
  • Loading branch information
nwoltman committed Apr 10, 2018
1 parent 908275f commit 9f81046
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 12 deletions.
22 changes: 15 additions & 7 deletions lib/Response.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ const ResponsePrototype = {

var payload = serializeError({
error: statusCodes['' + statusCode],
message: error && error.message || '',
message: getErrorMessage(error, statusCode),
statusCode,
})

Expand Down Expand Up @@ -214,14 +214,14 @@ const ResponsePrototype = {
},
}

function runOnSendHooks(response, payload) {
if (response.route.onSendHooks === null || response._ranOnSendHooks) {
sendFinalPayload(response, payload)
function runOnSendHooks(res, payload) {
if (res.route.onSendHooks === null || res._ranOnSendHooks) {
sendFinalPayload(res, payload)
} else {
response._ranOnSendHooks = true
res._ranOnSendHooks = true
runHooks(
response.route.onSendHooks,
response,
res.route.onSendHooks,
res,
payload,
sendFinalPayload
)
Expand Down Expand Up @@ -306,6 +306,14 @@ function getErrorStatus(error) {
return 500 // Internal Server Error
}

function getErrorMessage(error, statusCode) {
if (statusCode >= 500 && statusCode <= 599 && process.env.NODE_ENV === 'production') {
return '5xx Error'
}

return error && error.message || ''
}

// Aliases
ResponsePrototype.appendHeader = ResponsePrototype.append
ResponsePrototype.getHeader = ResponsePrototype.get
Expand Down
64 changes: 64 additions & 0 deletions test/error-handler-production.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
'use strict'

const t = require('tap')
const test = t.test
const medley = require('..')

process.env.NODE_ENV = 'production'

test('default error handler with 5xx status code - production', (t) => {
t.plan(8)

const app = medley()

app.get('/500', (req, res) => {
res.error(new Error('kaboom'))
})

app.get('/503', (req, res) => {
res.error(503, new Error('uh oh'))
})

app.inject('/500', (err, res) => {
t.error(err)
t.strictEqual(res.statusCode, 500)
t.strictEqual(res.headers['content-type'], 'application/json')
t.strictDeepEqual(JSON.parse(res.payload), {
error: 'Internal Server Error',
message: '5xx Error',
statusCode: 500,
})
})

app.inject('/503', (err, res) => {
t.error(err)
t.strictEqual(res.statusCode, 503)
t.strictEqual(res.headers['content-type'], 'application/json')
t.strictDeepEqual(JSON.parse(res.payload), {
error: 'Service Unavailable',
message: '5xx Error',
statusCode: 503,
})
})
})

test('default error handler with 4xx status code - production', (t) => {
t.plan(4)

const app = medley()

app.get('/', (req, res) => {
res.error(400, new Error('uh oh'))
})

app.inject('/', (err, res) => {
t.error(err)
t.strictEqual(res.statusCode, 400)
t.strictEqual(res.headers['content-type'], 'application/json')
t.strictDeepEqual(JSON.parse(res.payload), {
error: 'Bad Request',
message: 'uh oh',
statusCode: 400,
})
})
})
10 changes: 5 additions & 5 deletions test/500s.test.js → test/error-handler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const t = require('tap')
const test = t.test
const medley = require('..')

test('default 500', (t) => {
test('default error handler', (t) => {
t.plan(4)

const app = medley()
Expand All @@ -28,7 +28,7 @@ test('default 500', (t) => {
})
})

test('custom 500', (t) => {
test('custom error handler', (t) => {
t.plan(6)

const app = medley()
Expand Down Expand Up @@ -81,7 +81,7 @@ test('.setErrorHandler() should throw if not passed a function', (t) => {
t.end()
})

test('encapsulated 500', (t) => {
test('encapsulated error handler', (t) => {
t.plan(10)

const app = medley()
Expand Down Expand Up @@ -127,7 +127,7 @@ test('encapsulated 500', (t) => {
})
})

test('custom 500 with hooks', (t) => {
test('custom error handler with hooks', (t) => {
t.plan(8)

const app = medley()
Expand Down Expand Up @@ -164,7 +164,7 @@ test('custom 500 with hooks', (t) => {
})
})

test('cannot set errorHandler after binding', (t) => {
test('cannot set error handler after server is listening', (t) => {
t.plan(2)

const app = medley()
Expand Down

0 comments on commit 9f81046

Please sign in to comment.