rspec/rspec-rails

Rack status code name changes breaks HttpStatusMatcher

darrenboyd opened this issue · 6 comments

darrenboyd commented

What Ruby, Rails and RSpec versions are you using?

Ruby version: 3.3.3
Rails version: 7.1.3.4
RSpec version: 3.13.0
Rack version: 3.1.3

Observed behaviour

Rack has recently changed the name of the 422 status code from unprocessable_entity to unprocessable_content. However, both names will still resolve if Rack's public API is being used. However, the HttpStatusMatcher is looking at the Rack Constant directly, instead of using the provided API.

My suggestion is for the code in have_http_status.rb that looks like (around line 218)...

def set_expected_code!
  @expected ||=
    Rack::Utils::SYMBOL_TO_STATUS_CODE.fetch(expected_status) do
      raise ArgumentError,
            "Invalid HTTP status: #{expected_status.inspect}"
    end
end

be changed to....

def set_expected_code!
  @expected ||= Rack::Utils.status_code(expected_status)
end

Here's the behavior of the Rack::Utils.status_code method...

irb> Rack::Utils.status_code(:unprocessable_content)
  => 422
irb> Rack::Utils.status_code(:unprocessable_entity)
  => 422
irb> Rack::Utils.status_code(:invalidcode)
(irb):4:in `<main>': Unrecognized status code :invalidcode (ArgumentError)
pirj commented

Nice catch. Would you like to submit a PR?

sarvesh-sankaranarayanan commented

@pirj @darrenboyd Shouldn't we actually ask developers to use the actual changed status code name as per rack-utils, instead of supporting the old name?

pirj commented

This is how the public rack api behaves.

Who knows, might be they’ll decide to change the status back?

How would you interpret the definition of the “symbol to status code” as referenced from the assert_response mapping description in Rails guides?

ioquatix commented

For what it's worth, Rack just follows the specifications as defined by various RFCs.

Relevant RFCs

Both of the following RFCs define 422 but with a one-word difference:

  • RFC 4918, "Proposed Standard", June 2007, "Unprocessable Entity"
  • RFC 9110, "Internet Standard", June 2022, "Unprocessable Content"

Potential Confusion: Even though RFC 9110 it is newer, it does not obsolete RFC 4918 (directly nor transitively).

Arguments in Favor of RFC 9110

  1. RFC 9110 is titled "HTTP Semantics" which is a direct fit for this library, whereas RFC 4918 is scoped to WebDAV.
  2. RFC 9110 is an Internet Standard while RFC 4918 is only a Proposed Standard.
  3. MDN uses RFC 9110 for the 422 status code
  4. RFC 9110 is newer (2022 vs. 2007)

RFC 9110: 422: Unprocessable Content

15.5.21. 422 Unprocessable Content
The 422 (Unprocessable Content) status code indicates that the server understands the content type of the request content (hence a 415 (Unsupported Media Type) status code is inappropriate), and the syntax of the request content is correct, but it was unable to process the contained instructions. For example, this status code can be sent if an XML request content contains well-formed (i.e., syntactically correct), but semantically erroneous XML instructions.

References

From hyperium/http#664.

  • 👍3
  • ❤️1
darrenboyd commented

Nice catch. Would you like to submit a PR?

The simplest possible PR is here:

#2765

@sarvesh-sankaranarayanan, yeah, I tend to prefer encouraging people to move forward with changes like this. However, this change is more about using Rack's API in a better way. It looks like the Rack developers are going to add a deprecation warning for this change, and then force people to move over a future version.

  • ❤️2
ioquatix commented

Nice work team!

  • ❤️1