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

Fixes GH-916 by falling back to the old HTTP 413 Error Code if not present #917

Merged
merged 4 commits into from
Sep 18, 2015

Conversation

micahr
Copy link
Member

@micahr micahr commented Sep 18, 2015

Add compatibility with Node 4 for Http Status Codes

Moving between Node 0.12 and 4 introduces different http status code
messages for the request body size is too large error (413). This should
normalize between the two, picking the one that is not undefined.

Fixes #916

Moving between Node 0.12 and 4 introduces different http status code
messages for the request body size is too large error (413). This should
normalize between the two, picking the one that is not undefined.
@micahr
Copy link
Member Author

micahr commented Sep 18, 2015

I'd like to get this out as 4.0.3, so no one else has issues with receiving a 500 instead of a 413.

@micahr micahr self-assigned this Sep 18, 2015
// Between Node 0.12 and 4 http status code messages changed
// RequestEntityTooLarge was changed to PayloadTooLarge
// this check is to maintain backwards compatibility
if (PayloadTooLargeError !== undefined) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, consider using process.version to switch on these error codes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I knew there had to be a way to look that up.

Now that I have this in here, I think it might be less brittle to use the Error names, since the http code names actually changed in iojs 3. And if someone is using Restify with iojs 3, we should not break that.

@yunong
Copy link
Member

yunong commented Sep 18, 2015

@micahr looks good, feel free to merge. Thanks for the quick fix.

micahr added a commit that referenced this pull request Sep 18, 2015
Fix: HTTP 413 status name (fixes #916)

Fixes #916 by falling back to the old HTTP 413 Error Code if not present
@micahr micahr merged commit c803dbb into 4.x Sep 18, 2015
@retrohacker retrohacker deleted the fix-node-4-tests branch April 27, 2017 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants