HTTP Server: incorrect error message if status code is set to a string (like ENOENT)

[nfriedly]

• Version: 6.5.0
• Platform: Darwin Nathans-MBP.lan 16.0.0 Darwin Kernel Version 16.0.0: Mon Aug 29 17:56:20 PDT 2016; root:xnu-3789.1.32~3/RELEASE_X86_64 x86_64
• Subsystem: http

https://github.com/nodejs/node/blob/master/lib/_http_server.js#L190 converts the http status code to a number and then validates that it's within an appropriate range, throwing an error if it is not. However the error message includes the converted value rather than the original value.

If you have app code that does something dumb like copying err.code to the http status code, and err.code happens to be, for example, "ENOENT", the error message that is thrown is:

RangeError: Invalid status code: 0
     at ServerResponse.writeHead (_http_server.js:192:11)
     at ServerResponse.writeHead (/home/vcap/app/node_modules/on-headers/index.js:55:19)
     at ServerResponse._implicitHeader (_http_server.js:157:8)
     at ServerResponse.OutgoingMessage.end (_http_outgoing.js:559:10)
     at ServerResponse.send (/home/vcap/app/node_modules/express/lib/response.js:205:10)
     at ServerResponse.json (/home/vcap/app/node_modules/express/lib/response.js:250:15)
     at app.use (/home/vcap/app/config/error-handler.js:38:28)
     at Layer.handle_error (/home/vcap/app/node_modules/express/lib/router/layer.js:71:5)
     at trim_prefix (/home/vcap/app/node_modules/express/lib/router/index.js:310:13)

A better error message in this situation would be

RangeError: Invalid status code: ENOENT

I'll have a PR with a patch and a test for this shortly.

[fundon]

I found most libraries will to custom HTTP Error to cover that.

Like below:

function CustomError (code, message) {
   this.code = code
   this.status = this.statusCode = code in http.STATUS_CODES ? code : 500 // if you want
   this.message = message || http.STATUS_CODES[this.status]
   this.name = this.constructor.name
   Error.captureStackTrace(this, this.constructor)
}

Is there better practice?

[nfriedly]

@fundon That looks good to me, but it's kind of orthogonal to this issue. Node.js still should give helpful error messages when the user (or library) gets something wrong.

[nfriedly]

Fixed in a4bb9fd