Throw on bad argument to res.status()

[Ykid]

TypeError: Cannot read property 'toString' of undefined
    at ServerResponse.writeHead (_http_server.js:205:44)
    at ServerResponse.writeHead (/Users/altitudelabs/Documents/Altitude_Labs/lynk-mobileapi/node_modules/compression/node_modules/on-headers/index.js:53:19)
    at ServerResponse._implicitHeader (_http_server.js:172:8)
    at ServerResponse.res.write (/Users/altitudelabs/Documents/Altitude_Labs/lynk-mobileapi/node_modules/compression/index.js:90:14)
    at ServerResponse.res.end (/Users/altitudelabs/Documents/Altitude_Labs/lynk-mobileapi/node_modules/compression/index.js:111:14)
    at ServerResponse.res.send (/Users/altitudelabs/Documents/Altitude_Labs/lynk-mobileapi/node_modules/express/lib/response.js:150:8)
    at ServerResponse.res.json (/Users/altitudelabs/Documents/Altitude_Labs/lynk-mobileapi/node_modules/express/lib/response.js:191:15)

This error message prompted me to inspect the variable I passed in res.json instead of res.statusCode

express@4.0.0

[dougwilson]

Hm, weird. I don't think Express has such a thing as res.statusCode(undefined). Can you provide the code that you are using to reproduce?

[dougwilson]

Ah, I think perhaps you just made a typo in the description and are probably talking about res.status(). Currently we rely on Node.js core to report an issue, but it does not (and it doesn't seem like they intend to change this behavior, saying if you send the function garbage, expect garbage back out per nodejs/node#3269 (comment)). You can see this in the following example server:

var http = require('http')
var server = http.createServer(function (req, res) {
  res.statusCode = undefined
  res.setHeader('Content-Type', 'application/json')
  res.end(JSON.stringify({ prop: 'val' }))
})
server.listen(3000)

We can certainly start restricting the values to res.status, though that will pidgin hole us for future versions of Node.js if they start accepting other things, but I guess we can try to do all the validations Node.js core should probably be doing :)

A PR is welcome to add the argument check!

[Ykid]

@dougwilson oops. you are right. It is res.status(). Thanks!

[joshuacaron]

I created a pull request to solve this where arguments of null or undefined to res.status throw an error. Sorry for all of the referenced messages this is my first time creating a pull request. I deleted my first fork and remade it but apparently the references are still there, I didn't know that would happen.

[elevenpassin]

@dougwilson Hey, I want to take this up, I need a little guidance on where the error is being emitted from and how I can fix it. Thanks.

[dougwilson]

Hi @buoyantair the pull request has already been made by another user, unfortunately.

[KhaledSamir]

Hi @dougwilson , if the issue is solved. Can you please close it?

[dougwilson]

The PR is pending open questions and has not yet been merged into any release line, so it's not actually closed. The open status of the issue is accurate. If you believe otherwise please outline why you think the issue should be closed and I can close it.

[KhaledSamir]

@dougwilson Yeah, I understand you. I meant if there is already pull request that was accepted so, I thought this PR should be closed then. But I'm new here so not sure exactly how you guys manage the releases.

Thank you for replying back. I'll keep looking for existing issues that I can help with. Let me know if there is something I have to know before starting on any issue.

[dougwilson]

Hi @KhaledSamir sorry if there is any confusion; this is not a PR, it is an issue. The PR that was made to resolve this issue has not yet been accepted, hence why the issue is still open. I hope that makes sense.

[KhaledSamir]

@dougwilson Yup gotcha!. Thank you, man!