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

Rack changed their status codes to IETF RFC 7231 #20757

Merged
merged 2 commits into from Aug 22, 2015

Conversation

@bquorning
Copy link
Contributor

bquorning commented Jul 2, 2015

Rails is calling the Rack::Utils.status_code method in several places, and calling that method with an unknown symbol will return the integer 500.

The valid symbols are defined by the names in Rack::Utils::HTTP_STATUS_CODES. And they changed in in rack/rack#754, released in Rack v1.6.0 (see also https://github.com/rack/rack/blob/master/HISTORY.md). This PR changes Rails’ documentation to match the new valid status symbols.

Here’s the relevant diff from that Rack PR:

     HTTP_STATUS_CODES = {
       100 => 'Continue',
       101 => 'Switching Protocols',
 @@ -590,7 +590,6 @@ def to_params_hash
       303 => 'See Other',
       304 => 'Not Modified',
       305 => 'Use Proxy',
-      306 => 'Reserved',
       307 => 'Temporary Redirect',
       308 => 'Permanent Redirect',
       400 => 'Bad Request',
 @@ -606,12 +605,11 @@ def to_params_hash
       410 => 'Gone',
       411 => 'Length Required',
       412 => 'Precondition Failed',
-      413 => 'Request Entity Too Large',
-      414 => 'Request-URI Too Long',
+      413 => 'Payload Too Large',
+      414 => 'URI Too Long',
       415 => 'Unsupported Media Type',
-      416 => 'Requested Range Not Satisfiable',
+      416 => 'Range Not Satisfiable',
       417 => 'Expectation Failed',
-      418 => 'I\'m a teapot',
       422 => 'Unprocessable Entity',
       423 => 'Locked',
       424 => 'Failed Dependency',
 @@ -625,7 +623,7 @@ def to_params_hash
       503 => 'Service Unavailable',
       504 => 'Gateway Timeout',
       505 => 'HTTP Version Not Supported',
-      506 => 'Variant Also Negotiates (Experimental)',
+      506 => 'Variant Also Negotiates',
       507 => 'Insufficient Storage',
       508 => 'Loop Detected',
       510 => 'Not Extended',
@claudiob

This comment has been minimized.

Copy link
Member

claudiob commented Jul 2, 2015

🍵 I'm gonna miss you 😭 😭 😭 😭

Even though the actual change did not happen in the Rails codebase, it might still break someone's app when updating to Rails 5.

I think it might be useful to add a note to the "Upgrade to Rails 5" guide too.

Also, it looks like the failing test is not caused by the commit.

@bquorning

This comment has been minimized.

Copy link
Contributor Author

bquorning commented Jul 2, 2015

@bquorning bquorning force-pushed the bquorning:http-status-codes-changed-in-rack branch from a7134f5 to 3e6e6b9 Jul 3, 2015
@bquorning

This comment has been minimized.

Copy link
Contributor Author

bquorning commented Jul 3, 2015

Added a commit with upgraded 4.2 release notes.

@ncalca

This comment has been minimized.

Copy link

ncalca commented Jul 3, 2015

👍

1 similar comment
@dasch

This comment has been minimized.

Copy link
Contributor

dasch commented Jul 3, 2015

👍

@rails rails locked and limited conversation to collaborators Jul 3, 2015
@rails rails unlocked this conversation Jul 26, 2015
@bquorning

This comment has been minimized.

Copy link
Contributor Author

bquorning commented Aug 13, 2015

Ping @spastorino who merged rack/rack#754 into Rack master. Sorry if pinging is considered rude 😊

@spastorino
spastorino reviewed Aug 14, 2015
View changes
guides/source/4_2_release_notes.md Outdated
@@ -227,6 +227,17 @@ restore the old behavior.
If you do this, be sure to configure your firewall properly such that only
trusted machines on your network can access your development server.

### Changed status option symbols for `render`

Due to a [change in Rack](https://github.com/tonyta/rack/commit/be28c6a2ac152fe4adfbef71f3db9f4200df89e8), the symbols that the `render` method accepts for the `:status` option have changed:

This comment has been minimized.

Copy link
@spastorino

spastorino Aug 14, 2015

Member

You should point to the rack repository here

This comment has been minimized.

Copy link
@bquorning

bquorning Aug 21, 2015

Author Contributor

Whoops, you’re right.

@spastorino

This comment has been minimized.

Copy link
Member

spastorino commented Aug 14, 2015

I think we should revert this in Rack 1.6.x and release again and apply only to 2.x. For some reason I thought it was only applied to 2.x.

@bquorning

This comment has been minimized.

Copy link
Contributor Author

bquorning commented Aug 21, 2015

It was released in Rack 1.6.0 more than 8 months ago. Isn’t it a bit too late to revert a breaking change?

My guess is that a lot of applications have already been changed to work around this change. It would be confusing if they had to change the behavior back the next time they upgrade Rack.

@bquorning bquorning force-pushed the bquorning:http-status-codes-changed-in-rack branch to 3ec3ac6 Aug 21, 2015
spastorino added a commit that referenced this pull request Aug 22, 2015
…-rack

Rack changed their status codes to IETF RFC 7231
@spastorino spastorino merged commit 4115a12 into rails:master Aug 22, 2015
@bquorning bquorning deleted the bquorning:http-status-codes-changed-in-rack branch Aug 22, 2015
@gamafranco

This comment has been minimized.

Copy link

gamafranco commented Aug 29, 2015

Mind me to update the integration with my PiTeapot...

@BenMorganIO

This comment has been minimized.

Copy link
Contributor

BenMorganIO commented Sep 1, 2015

Wait, I use I'm a Teapot! :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.