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

Ensure status param takes precedence over error.status #38

Closed
wants to merge 1 commit into from
Closed

Ensure status param takes precedence over error.status #38

wants to merge 1 commit into from

Conversation

jasonkarns
Copy link

I believe this is a bug.

Prior to this change:

_err = new Error()
_err.status = 404
err = createError(500, _err) // err.status == 404

After this change (no tests modified):

_err = new Error()
_err.status = 404
err = createError(500, _err) // err.status == 500

@dougwilson
Copy link
Contributor

Imo it is not a bug, orgerwide if you wrap an error from thos module with a different code, the error name and other props will not reflect correctly.

@dougwilson
Copy link
Contributor

I.e. you can end up with err instanceof NotFoundError but err.statusCode === 410 witch is not desired.

@jasonkarns
Copy link
Author

Closing as works-as-designed.

#37 now includes a new test to actually verify/document this behavior as intended. 91a7445#diff-c1129c8b045390789fa8ff62f2c6b4a9R180

@jasonkarns jasonkarns closed this Sep 8, 2017
@jasonkarns jasonkarns deleted the status-param-over-error branch September 8, 2017 14:13
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.

2 participants