Response reason isn't respected

[bryanp]

Setting the response reason for Async::HTTP::Protocol::Response does not seem to work:

require "async/http/server"
require "async/reactor"
require "async/http/endpoint"
require "async/http/protocol/response"

endpoint = Async::HTTP::Endpoint.parse("http://127.0.0.1:9294")

app = lambda do |request|
  Async::HTTP::Protocol::Response.new(nil, 200, "Foo")
end

server = Async::HTTP::Server.new(app, endpoint)

Async::Reactor.run do
  server.run
end

Here's the response:

$ curl -v http://127.0.0.1:9294
* Rebuilt URL to: http://127.0.0.1:9294/
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 9294 (#0)
> GET / HTTP/1.1
> Host: 127.0.0.1:9294
> User-Agent: curl/7.54.0
> Accept: */*
>
< HTTP/1.1 200 With Honour.
< content-length: 0
<
* Connection #0 to host 127.0.0.1 left intact

I would expect to see HTTP/1.1 200 Foo.

[ioquatix]

Yeah, we should probably respect it. The problem is, if it's nil, it breaks Safari WebSockets. Also, the concept of "Reason" was completely removed from HTTP/2, so I thought about actually removing it from Protocol::HTTP::Response.

What do you think?

[bryanp]

I think we should keep support in Protocol::HTTP::Response just for HTTP/1 completeness. Maybe don't allow it to be nil and default to Unknown?

[ioquatix]

I guess I wonder is there any reason why anyone would want to set it in normal usage, given that at a guess, a large percentage of requests would never use it?

We can have a lookup table for normal status codes, and use the strings according to RFCs, e.g.

REASONS = {
  200 => "Okay"
}

and so on.

The only reason why I exposed it is because it seemed interesting - you could use it as a side channel for data. But it's HTTP/1 specific, so if someone wanted to use that, maybe they'd just use protocol-http1 directly.

[bryanp]

I've been thinking on this and agree with your reasoning. Maybe something like this for the lookup:

• connection/statuses.rb

When should the reason argument be removed from Async::HTTP::Protocol::Response? Since it's pre 1.0 we don't care too much about backwards compatibility, but you may have an opinion here.

[ioquatix]

That's useful, I'll use that list. I'll remove it in the next release.

[ioquatix]

Okay, it was released in v0.43.0.

[ioquatix]

Thanks for the list it probably saved me at least an hour of mucking about with RFCs.

[ioquatix]

If you want to have one central place for list, feel free to use Protocol::HTTP1::Reason::DESCRIPTIONS.