Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw error if argument to res.status is null or undefined #2797

Closed
wants to merge 1 commit into from

Conversation

joshuacaron
Copy link
Contributor

Fixes #2795 where res.status gives undescriptive error when the argument is null or undefined. I tested and all other inputs don't seem to have the same issue as null and undefined since they have .toString methods. I added unit tests to verify the new behaviour of res.status.

This is my first pull request on an open source project so if I formatted something wrong or went about submitting this wrong please let me know.

@ebramanti
Copy link

Hope this gets merged, just ran into this issue yesterday!

@dougwilson
Copy link
Contributor

Hi @jadengore , this is a breaking change and is tagged for the 5.x series.

@dougwilson
Copy link
Contributor

Ah, yea, just looked at it and I see now. This is actually the third open PR to add this check. Maybe the other one has the code number check?

@dougwilson
Copy link
Contributor

Sorry, my above comment was made in the wrong PR

@czaarek99
Copy link
Contributor

czaarek99 commented Apr 15, 2018

If we're adding error checking here, is there a reason why we're not just enforcing an integer? If the change is breaking we might as well go all the way?

EDIT: This pull request makes more sense by me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Throw on bad argument to res.status()
4 participants