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

Add rack.early_hints to SPEC #1831

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jeremyevans
Copy link
Contributor

This is already de facto spec as both Unicorn and Puma
implement it. The changes to SPEC are compatible with
both implementations.

Fixes #1692
Fixes #1695

Co-authored-by: Jeremy Evans code@jeremyevans.net

@ioquatix
Copy link
Member

ioquatix commented Mar 16, 2022

I want to find a way to move forward here but I have a hard time accepting this interface because even if not initially apparent, it does introduce a lot of downstream complexity which I'm not sure we are prepared to deal with.

Just to confirm, the RFC is still marked as experimental right? https://httpwg.org/specs/rfc8297.html

When I look at a feature like this, I have to consider how it fits into async-http which is what Falcon is based on. async-http has symmetrical handling of client and server requests/responses. This means any additional interface complexities have to be accurately captured throughout the client request, server request, server response, client response.

Early hints are effectively multiple responses per request which is very uncommon in normal HTTP (at least in my assumptions about what typical developers think about HTTP). Sure, you can implement it as a hook on the incoming request (as proposed), but it's actually a feature of the response. Architecturally this is a huge problem, since async-http has a semantic model which allows transparent proxying of requests and responses even over protocol versions, by having a standard representation of these objects.

Early hints make this representation significantly more complex. A practical example of this is how Falcon works internally when handling virtual hosts. Falcon has a front end proxy which talks SSL + HTTP/1 or HTTP/2. Internally, it talks to the application server using plain text HTTP/2 via a unix domain socket. If the application server replies with a 1xx provisional response, the proxy layer has to understand this and correctly send it to the client. This requires early hints / provisional responses to become a first class concept within the semantic request/response models, and any code that deals with transiting requests/responses needs to be aware of those new interfaces. This is the complexity which I'm not comfortable with, without seeing significant performance advantages. The same logic applies to push promises which is why I wouldn't mind removing them simply because I'm not sure the additional complexity yields any practical impact.

Therefore, the bar for supporting this functionality should exceed the additional complexity both at the semantic level (how we model requests and responses) and the user level (how users are expected to generate and consume these extra early hints). That bar for me is pretty high, and I'm still not sure the performance advantage.

Does the cost of the complexity yield a significant performance advantage?

By standardising this in Rack, we are sending a message: this is an important part of the HTTP semantic model that we allow you to use. However, the net result might be a fair amount of disappointment as people find the performance is less advantageous than expected. And the net result is a huge amount of technical complexity we now have to carry forward, both on the server side and client side.

I'm happy to experiment with this further, but I hesitate to standardise it without evidence that it provides a significant advantage, especially given that there is a more general interface (provisional response) which fully captures the actual semantic of HTTP. I'd be more willing to consider the provisional response interface, but even then I'd like to see the performance advantage. As existing servers already support it, I don't see why it can't be tested in the real world with real data collection to show it's impact. Standardisation in rack does not appear to me to be a barrier to this testing.

@ioquatix
Copy link
Member

Just for my own edification I was reading https://blog.cloudflare.com/early-hints/ and felt it summarised some key issues nicely:

Standards like Early Hints often don’t get off the ground because of this chicken and egg problem: clients have no reason to support new standards without server support and servers have no reason to implement support without clients speaking their language. As previously discussed, Early Hints is particularly complicated for origins to directly support, because 103 is an unfamiliar status code and multiple responses to a single request may not be a well-supported pattern in common HTTP server and application stacks.

It also outlines some steps they have tried to address those problems.

@ioquatix
Copy link
Member

Do any browsers support this on HTTP/1? It looks like this isn't supported on HTTP/1 by most common browsers?

@jeremyevans
Copy link
Contributor Author

I want to find a way to move forward here but I have a hard time accepting this interface because even if not initially apparent, it does introduce a lot of downstream complexity which I'm not sure we are prepared to deal with.

First, this is purely an optional part of Rack, so servers do not have to implement it if they don't want to accept the complexity. From looking at the Unicorn and Puma implementation, the complexity required is not trivial, but it's not large either.

Does the cost of the complexity yield a significant performance advantage?

This should be a decision made by each web server. Servers that don't think the performance advantage outweigh the complexity disadvantages should not implement it. Again, this is a purely optional part of the spec, being formally standardized as it is already the de facto standard.

I get for Falcon/async-http, supporting early hints may not be worth it. I think it's fine not to support it. After all, it is optional, not required, by design.

By standardising this in Rack, we are sending a message: this is an important part of the HTTP semantic model that we allow you to use. However, the net result might be a fair amount of disappointment as people find the performance is less advantageous than expected. And the net result is a huge amount of technical complexity we now have to carry forward, both on the server side and client side.

No. By standardizing this as an optional part of rack, we are sending a message that if you want to use this feature as a server or as a framework, this is the interface you use. This is no recommendation that you should use it, merely a specification of the interface to use it for the server/framework to use.

I'm happy to experiment with this further, but I hesitate to standardise it without evidence that it provides a significant advantage, especially given that there is a more general interface (provisional response) which fully captures the actual semantic of HTTP. I'd be more willing to consider the provisional response interface, but even then I'd like to see the performance advantage. As existing servers already support it, I don't see why it can't be tested in the real world with real data collection to show it's impact. Standardisation in rack does not appear to me to be a barrier to this testing.

I don't understand this logic. Everything you seem to be against with early hints would also apply to provisional responses.

While there are use cases for early hints (even if you don't think they are that useful), I have not seen a single use case of provisional responses other than for early hints. If in some theoretical future, there was actually a use for provisional responses, we could implement rack.early_hints on top of it easily. YAGNI definitely applies to provisional responses, and until that is not the case, and provisional responses have a use case beyond early hints, it doesn't make sense to standardize provisional responses.

rack.early_hints is already supported by multiple web servers (Puma, Unicorn, Falcon) and multiple web frameworks (Rails, Roda, Hanami). If we standardize provisional responses and not the de facto standard of early hints, we will be making life difficult for all web servers and web frameworks that implement early hints, for absolutely no current benefit.

Do any browsers support this on HTTP/1? It looks like this isn't supported on HTTP/1 by most common browsers?

Chrome doesn't support it on HTTP 1.1: https://chromium.googlesource.com/chromium/src/+/master/docs/early-hints.md

@ioquatix
Copy link
Member

I'm only slightly more positive towards provisional responses, but at least it's the actual semantic model that's exposed at the protocol level. I don't agree with your YAGNI assessment.

Standardising it in Rack is we make it a defacto standard which can then make it harder to later implement better interfaces.

Frankly, if we want to provide an interface for informing browsers that they should download specific files, I don't think this is a good one.

I think it would make more sense to write it like this:

env['rack.early_hints'].call(path, **attributes)
# map to something like link: #{path}; #{attributes.join(';')}

With such an interface, mapping to early hints, push promises or anything else becomes much more trivial. If you are arguing that a specific interface here makes sense, I don't think a generic list of headers make sense.

@jeremyevans
Copy link
Contributor Author

I'm only slightly more positive towards provisional responses, but at least it's the actual semantic model that's exposed at the protocol level. I don't agree with your YAGNI assessment.

Can you provide a single use case for a provisional response other than early hints? If not, YAGNI (not yet, at least).

Standardising it in Rack is we make it a defacto standard which can then make it harder to later implement better interfaces.

rack.early_hints is already a de facto standard, because that's what web servers implement and what web frameworks use. Us standardizing rack.early_hints takes it from a de facto standard to a de jure standard.

Frankly, if we want to provide an interface for informing browsers that they should download specific files, I don't think this is a good one.

I think it would make more sense to write it like this:

env['rack.early_hints'].call(path, **attributes)
# map to something like link: #{path}; #{attributes.join(';')}

With such an interface, mapping to early hints, push promises or anything else becomes much more trivial. If you are arguing that a specific interface here makes sense, I don't think a generic list of headers make sense.

I'm making the same argument made by @tenderlove at #1701 (comment):

If popular web servers all do it, then it's really the defacto spec. Rack's job is to define the interface, and if all web servers agree this is a fine interface then I think we're good to go?

IMO, neither the provisional response interface nor the modified interface in your comment is better enough that we should ask all existing implementations to change their code.

@ioquatix
Copy link
Member

Can you provide a single use case for a provisional response other than early hints? If not, YAGNI (not yet, at least).

"100 Continue"

@jeremyevans
Copy link
Contributor Author

Can you provide a single use case for a provisional response other than early hints? If not, YAGNI (not yet, at least).

"100 Continue"

That's a status code, not a use case. Can you explain how/why you would use provisional responses in an application with status code 100?

@ioquatix
Copy link
Member

100 continue can be used to inform the client that authorisation header was successful and they can continue to upload the request body.

@jeremyevans
Copy link
Contributor Author

100 continue can be used to inform the client that authorisation header was successful and they can continue to upload the request body.

I'll give you that that is at least a theoretical use case, though I'm not sure there is any software that actually does that.

In any case, I've said my piece. If the fact that existing servers and frameworks already use rack.early_hints is not sufficient evidence this should be merged, it doesn't seem productive to continue to attempt to persuade.

@tenderlove @rafaelfranca @matthewd I'd appreciate if one of you could give your opinion on whether or not this should be merged.

@ioquatix
Copy link
Member

ioquatix commented Mar 17, 2022

I'll give you that that is at least a theoretical use case.

I've read about it being used to prevent large uploads beings sent before auth is performed, e.g. https://support.airship.com/hc/en-us/articles/213492003--Expect-100-Continue-Issues-and-Risks

https://evertpot.com/http/100-continue/ also has a great overview of different use cases for 100 continue, 102 processing and so on.

So not just theoretical.

Copy link
Collaborator

@rafaelfranca rafaelfranca left a comment

Choose a reason for hiding this comment

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

I'm with @jeremyevans and @tenderlove on this. If most web frameworks and most popular ruby servers already implement this feature it is already a de-factor standard of rack.

I don't see we removing this feature with push back from the frameworks. I also don't see we changing this feature without taking in consideration the actual usage.

In my opinion we should add to the spec as an optional part of Rack.

@ioquatix
Copy link
Member

ioquatix commented Apr 6, 2022

After thinking about this, I'm ultimately fine with it, but I still believe it's a mistake to introduce a specific interface when a more general one exists (and has valid use cases). It sets the bar incorrectly for framework and middleware authors IMHO which creates long term technical debt; and there is also a performance cost to servers setting additional env hash keys which are not used. This isn't about what servers support (we don't change that nor can the spec really change that), but about what we want to standardise for consumers and clients of Rack.

I'd personally prefer we implement rack.provisional_response. I will reserve my judgement but would like to hear @tenderlove's opinion.

@jeremyevans
Copy link
Contributor Author

Just for my own edification I was reading https://blog.cloudflare.com/early-hints/ and felt it summarised some key issues nicely:

Standards like Early Hints often don’t get off the ground because of this chicken and egg problem: clients have no reason to support new standards without server support and servers have no reason to implement support without clients speaking their language. As previously discussed, Early Hints is particularly complicated for origins to directly support, because 103 is an unfamiliar status code and multiple responses to a single request may not be a well-supported pattern in common HTTP server and application stacks.

The chicken and egg problem has been partly solved, as Chrome 103 supports Early Hints. @tenderlove what are your thoughts on this issue?

@ioquatix
Copy link
Member

"Supports" may be a bit too strong.

A few months ago, we announced a collaborative experiment [1] to measure the potential impact of 103 early hints. The results indicate that Early Hints could result in speed improvements on the order of a few hundred milliseconds (the sample size is a bit small though). Given these encouraging results, we are now moving forward with the next steps, starting with this intent to prototype.

They are testing a prototype and it can be removed in a future release. I've seen this kind of thing before and more likely than not it will end up being removed.

As I said earlier my position is we should support a generic interface like rack.provisional_response. Because that's the semantic model of HTTP, I have no problem adding that to the spec.

@jeremyevans
Copy link
Contributor Author

@ioquatix I'm not sure if you are aware, but it looks like your quote is from February 2021, when they were at the testing/evaluation stage (https://groups.google.com/a/chromium.org/g/blink-dev/c/DAgWIczGtG0/m/gSXvjYn-AwAJ). Your quote does not reflect the current status in Chrome.

The release announcement for Chrome 103 (https://developer.chrome.com/blog/new-in-chrome-103/) has Early Hints as the most important new feature in the release. It's not beyond the realm of possibility that they will remove it, but there is no language indicating they are at the testing/evaluating stage. Unless you have a better current source, it appears Early Hints is fully supported for production use as of Chrome 103.

@ioquatix
Copy link
Member

I was referring to this page: https://chromestatus.com/feature/5207422375297024

@ioquatix
Copy link
Member

ioquatix commented Jun 23, 2022

By the way, my position doesn't change much, but I'm really excited by this feature. I still think rack.provisional_response is the correct interface.

My reasoning is:

  • We should try to support the HTTP semantics as directly and without color/opinion.
  • I see early hints as a subset of provisional response.
  • I don't think we'd implement rack.early_hints if we already had rack.provisional_response.
  • I understand servers support it and that won't change whether we add a spec for it or not.
  • Adding a spec for either rack.early_hints and/or rack.provisional_response does not depend on each other of course.
  • The question becomes, do we want both? Because there is a cost to adding more hash keys for every request.
  • There are potentially other good use cases for provisional responses (other than early hints).
  • Therefore, I'd prefer we just have the more generic option.

I'm not against anyone using early hints, I personally think it's an interesting idea - but there are a ton of challenges with provisional responses because they essentially mean that one HTTP request has multiple responses and that's unexpected for 99% of use cases. The counter-point is simply being able to send link headers in the initial headers, and streaming the response body, which is effectively the same in terms of what kind of performance you should be able to get by hinting the browser what resources to load.

Another counter point is HTTP/2 server push which is essentially dead. It's very very similar to early hints, except that instead of being browser initiated, the server initates the stream and pushes the resources it thinks the browser will need. This would be one data point as to why I'm not sure if early hints really has a long term future (this and the ergonomics of handling multiple responses).

@jeremyevans
Copy link
Contributor Author

I was referring to this page: https://chromestatus.com/feature/5207422375297024

Considering the language is the same as the February 2021 blog post, it is likely stale. This was tested in Origin trails from Chrome 94 to 102. It seems unlikely to me that after testing it in 9 versions, and deciding it is good enough to enable by default, that they would remove it.

It's not just servers that support rack.early_hints. Rails, Roda, and Hanami all already support rack.early_hints as well, as mentioned previously.

Anyway, seems pointless to rehash the same discussion that we had earlier in the issue. My point in updating this issue was to add new information not previously discussed (chicken and egg problem solved, now fully supported in Chrome 103!), not to have the same discussion again.

@ioquatix ioquatix added this to the v3.1.0 milestone Jun 25, 2022
@jeremyevans jeremyevans modified the milestones: v3.1.0, v3.0.0 Aug 13, 2022
@jeremyevans
Copy link
Contributor Author

As @tenderlove, @rafaelfranca, and I are in favor of this, I plan on merging this in a couple days. I think this and rack.response_finished (#1802) are the last features planned for Rack 3 (according to https://github.com/rack/rack/projects/2), so after merging, we can probably put out an rc release.

CHANGELOG.md Show resolved Hide resolved
@ioquatix
Copy link
Member

ioquatix commented Aug 14, 2022

Are you also happy with adding a spec for an optional rack.provisional_response and/or rack.push_promise?

@jeremyevans
Copy link
Contributor Author

Are you also happy with adding a spec for an optional rack.provisional_response and/or rack.push_promise?

Not at present.

@ioquatix
Copy link
Member

I want to get input from server authors before this is merged. @FooBarWidget @nateberkopec

I went looking through the original discussion and @tenderlove seemed to be in agreement with adding the status code: #1692 (comment) however maybe his position is updated/changed. @tenderlove can we get your input on this?

If I ever decided to implement something to this effect in falcon, I'd probably prefer to introduce rack.provisional_response(status, headers). I know current servers support rack.early_hints(headers), but I'd like to hear from the server authors whether this is the interface they want to support, or if they want to give slightly more control to their users.

As I've already explained, it doesn't make sense to support both interfaces when one is a strict superset of the other, so we should be careful about what we introduce and what that communicates to our users. It would be nice to support 100 Continue and other headers without needing two different optional specifications.

@jeremyevans
Copy link
Contributor Author

One difference between rack.early_hints and rack.after_reply/rack.response_finished is that rack.early_hints, in addition to being supported by multiple popular web servers, is also already supported by popular frameworks (at least Rails, Roda, and Hanami). Switching from rack.early_hints to rack.provisional_response would be breaking a lot of backwards compatibility for only theoretical future benefit. YAGNI, IMO. I too await @tenderlove's decision here.

Let's examine the links you posted previously in favor of your argument:

I've read about it being used to prevent large uploads beings sent before auth is performed, e.g. https://support.airship.com/hc/en-us/articles/213492003--Expect-100-Continue-Issues-and-Risks

We recommend against the use of Expect: 100-Continue.

https://evertpot.com/http/100-continue/ also has a great overview of different use cases for 100 continue, 102 processing and so on.

Regarding 100:

My personal experience with 100 Continue is mostly confusion and frustration. ... This broke a
number of obscure clients I had to deal with for a project I was working on.

Regarding 102:

102 Processing is a pretty rare status code. It’s not in the official newer HTTP specifications, and its
only appearance is in an obsolete version of the WebDAV specification. It was left out of the current
WebDAV specification, and more recent HTTP specifications.

@ioquatix
Copy link
Member

ioquatix commented Aug 17, 2022

Let's examine the links you posted previously in favour of your argument.

My personal experience with 100 Continue is mostly confusion and frustration. ... This broke a
number of obscure clients I had to deal with for a project I was working on.

Will apply to all 1xx status codes.

I was interested in this so I just whipped up a fake 103 server: https://github.com/socketry/protocol-http1/blob/main/examples/early-hints/server.rb

At least, libcurl and Net::HTTP seem to be okay with it, but there is no way to see the 103 status that I can see from Net::HTTP's interface. Doing a survey of different clients might be useful, or at least provide some insight into "broke a number of obscure clients".

This is already de facto spec as Unicorn, Puma, and Falcon
implement it. The changes to SPEC are compatible with
both implementations.

Fixes rack#1692
Fixes rack#1695

Co-authored-by: Jeremy Evans <code@jeremyevans.net>
@jeremyevans
Copy link
Contributor Author

@tenderlove I've rebased this against current master. Can you approve (or request changes)? This is the last blocker for Rack 3, AFAIK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add spec for early hints
5 participants