Skip to content

Update HTTP status codes and associated symbols#2137

Merged
ioquatix merged 4 commits into
rack:mainfrom
wtn:revise_codes
Dec 7, 2023
Merged

Update HTTP status codes and associated symbols#2137
ioquatix merged 4 commits into
rack:mainfrom
wtn:revise_codes

Conversation

@wtn

@wtn wtn commented Nov 24, 2023

Copy link
Copy Markdown
Contributor

These commits bring Rack in sync with the 2022-06-08 version of the IANA HTTP Status Code Registry.

This change is notably impactful in that the symbol for HTTP status code 422 changes from :unprocessable_entity to :unprocessable_content. To mitigate the impact, I added legacy symbol lookup to Rack::Utils.status_code.

It seems that some Rubyists are in the habit of retrieving status codes from Rack::Utils::SYMBOL_TO_STATUS_CODE directly (examples: here, here, here) rather than using the public method. Admittedly, I've done this myself. Well, it can't be helped, but perhaps this pattern can be discouraged in the future.

@ioquatix

Copy link
Copy Markdown
Member

This looks reasonable to me. @jeremyevans are you happy that backwards compatibility is sufficient?

@jeremyevans jeremyevans left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm good with the general idea.

Comment thread lib/rack/utils.rb Outdated
Comment thread lib/rack/utils.rb Outdated
@wtn

wtn commented Nov 25, 2023

Copy link
Copy Markdown
Contributor Author

I removed the duplicated constant and added deprecation warnings.

The use of Hash#invert here is not ideal, but I didn't want to add yet more constants.

Comment thread lib/rack/utils.rb

@ioquatix ioquatix left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking good, just a few minor suggestions.

Comment thread lib/rack/utils.rb Outdated
Comment thread lib/rack/utils.rb Outdated
Comment thread lib/rack/utils.rb Outdated
Comment thread lib/rack/utils.rb Outdated
@wtn

wtn commented Nov 27, 2023

Copy link
Copy Markdown
Contributor Author

Thank you for feedback. I've made the following changes:

  1. use Hash#fetch in fallback symbol lookup
  2. rename OBSOLETE_HTTP_REASON_PHRASES to OBSOLETE_SYMBOLS_TO_STATUS_CODES and make it private
  3. add private constant OBSOLETE_SYMBOL_MAPPINGS to precompute preferred symbol for deprecation message
  4. use ternary statement for deprecation warning for succinctness and readability

@wtn wtn requested a review from ioquatix November 27, 2023 22:06

@jeremyevans jeremyevans left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks almost ready. I just have a couple of minor change requests.

Comment thread test/spec_utils.rb
Comment thread lib/rack/utils.rb Outdated

@ioquatix ioquatix left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, let's see if @jeremyevans is okay to merge.

@ioquatix ioquatix merged commit 64ad26e into rack:main Dec 7, 2023
@wtn wtn deleted the revise_codes branch December 8, 2023 05:06
@rafaelfranca

Copy link
Copy Markdown
Collaborator

Can we introduce the new names but keep the old names not deprecated?

For a libraries on top of rack the only way to support Rack 2 and Rails 3 at the same time after this change is by introducing a bunch of conditionals on the version of rack.

@ioquatix

ioquatix commented Jan 4, 2024

Copy link
Copy Markdown
Member

@rafaelfranca feel free to create a PR, not completely sure what issue you are having but sounds reasonable.

@rafaelfranca

Copy link
Copy Markdown
Collaborator

Will do.

The problem is that in Rails we have a few controllers with head :unprocessable_entity. To work without deprecation on Rack 3 we need to change it to head :unprocessable_content, but that doesn't work on Rack 2. So to work on both version we need to change the code to be:

head RACK_VERSION >= "3" ? :unprocessable_content : :unprocessable_entity

or some variation of that.

I'm also considering dropping support to Rack 2, in that case the problem will be solved for Rails 8, but not for 7.1, but I can probably work around the issue itself in Rails since that code will not live for much loger.

@ioquatix

ioquatix commented Jan 4, 2024

Copy link
Copy Markdown
Member

I believe you can use the status_code helper? Maybe it resolves the issue for you.

@rafaelfranca

rafaelfranca commented Jan 5, 2024

Copy link
Copy Markdown
Collaborator

That shows the deprecation. We don't want Rails to show people deprecation message for code the framework has.

@wtn

wtn commented Jan 5, 2024

Copy link
Copy Markdown
Contributor Author

In my view, it's best to keep the deprecation message in Rack.

Frameworks like Rails can manage the transition by adding status code symbols:

if Gem::Version.new(Rack.release) >= Gem::Version.new('3.y.z')
  Rack::Utils::SYMBOL_TO_STATUS_CODE[:payload_too_large] = 413
  Rack::Utils::SYMBOL_TO_STATUS_CODE[:unprocessable_entity] = 422
end

SYMBOL_TO_STATUS_CODE is not frozen, consider it part of the public API.

@rafaelfranca

rafaelfranca commented Jan 5, 2024

Copy link
Copy Markdown
Collaborator

This is exactly my point. I don't think rack should be pushing frameworks to do version checks. Rack is supposed to be a stable API. If a framework that depends on rack needs to check a version of the library we can't consider Rack stable anymore.

I'm all for adding features to Rack, but all their changes need to be backward compatible in a way that users don't have a hard time supporting 2 versions of Rack.

So my suggestion is: while Rack 2 is still supported, we don't show any deprecation, when only Rack 3 is supported we can add the deprecation in some Rack 3.x and in Rack 4 we remove it.

If this plan was hard to implement, I'd totally understand pushing version checks to users, but I'm sure we can keep an old status code name longer to make users' life easier.

@rafaelfranca

Copy link
Copy Markdown
Collaborator

Just to be clear, not criticizing the breaking changes/deprecations we had to do to evolve the spec, fix issues, etc, just saying there are a few like this one we can avoid if we decide to.

@tenderlove

Copy link
Copy Markdown
Member

SYMBOL_TO_STATUS_CODE is not frozen, consider it part of the public API.

I don't think we should consider mutating another library's constants to be "part of the public API". It's true that the constant is not frozen, but I also consider it bad manners to mutate someone else's constants. Also, in general, I think it's acceptable to freeze a constant but not do a major version release. IOW constant mutation is not public API, please do not consider it as such. If we want an API to mutate this constant, then we should add methods people call to register keys and values.

So my suggestion is: while Rack 2 is still supported, we don't show any deprecation, when only Rack 3 is supported we can add the deprecation in some Rack 3.x and in Rack 4 we remove it.

If this plan was hard to implement, I'd totally understand pushing version checks to users, but I'm sure we can keep an old status code name longer to make users' life easier.

I think this is reasonable.

morgoth added a commit to tiramizoo/rails that referenced this pull request Jun 11, 2024
…processable_content"

This fixes rack deprecation warning

rack/rack#2137
@tisba

tisba commented Jun 17, 2024

Copy link
Copy Markdown

This seems to cause issues to dependencies like Rails (fixed in #2209) but also RSpec, see rspec/rspec-rails#2763.

kakeru-one added a commit to Progaku-copy/progaku-archive that referenced this pull request Sep 10, 2024
Ref: rack/rack#2137

Rackをupdateしたため、その対応が必要になった
goulvench added a commit to betagouv/collectif-objets that referenced this pull request Oct 18, 2024
goulvench added a commit to betagouv/collectif-objets that referenced this pull request Oct 18, 2024
drjayvee added a commit to drjayvee/rubygems.org that referenced this pull request Aug 4, 2025
simi pushed a commit to rubygems/rubygems.org that referenced this pull request Aug 4, 2025
@jrochkind

Copy link
Copy Markdown
Contributor

I don't quite undertand why I started getting deprecation messages only on upgrading to Rack 3.2 -- did not get them in Rack 3.1, did get them in Rack 3.2.

Is that the intent? Reading the comments here, it does not seem to be the intent, but it's what's going on for me?

On Rails 8.0.2.1 code like return render plain: "Error: unpermitted parameters.", status: :unprocessable_entity.

Deprecation notice in Rack 3.2, not in Rack 3.1.

@ioquatix

ioquatix commented Sep 9, 2025

Copy link
Copy Markdown
Member

IIRC, we decided to not emit deprecation warnings in 3.1 because it was too noisy, to give Rails a chance to do something about it.

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.

7 participants