Add early hints feature #1403

eileencodes · 2017-08-29T17:54:56Z

Worked with @tenderlove on adding early hints. This is still a work in progress as there are a few things we need to address, but this PR is to start the conversation.

This commit adds the early hints lambda to the headers hash. Users can
call it to emit the early hints headers. For example:

class Server
  def call env
    if env["REQUEST_PATH"] == "/"
      env['rack.early_hints'].call([{ link: "/style.css", as: "style" }, { link: "/script.js" }])
      [200, { "X-Hello" => "World" }, ["Hello world!"]]
    else
      [200, { "X-Hello" => "World" }, ["NEAT!"]]
    end
  end
end

run Server.new

In this example, the server sends stylesheet and javascript early hints
if the proxy supports it, it will send H2 pushes to the client.

Of course not every proxy server supports early hints, so to enable the
early hints feature with puma you have to pass the configuration variable,
--early-hints.

tenderlove · 2017-08-29T18:03:51Z

@eileencodes and I are working on pushing early hints support in to the Rack spec for version 2 (version 2 of the SPEC, not version 2 of the gem). I'd like to try experimentally implementing this in Puma first, then push support to Rails, then upstream to the Rack SPEC. IOW, I don't want this to be done in a vacuum.

Does this seem like an acceptable approach for the Puma team? I can understand if you don't want to be used for experimentation, but I would like to get this in to production systems sooner rather than later. 😊

Also, we've verified this to work with Puma + h2o.

tenderlove · 2017-08-29T18:04:38Z

I think we're using Ruby 2.3+ specific features in the test in this PR. If the overall concept seems OK, then we'll fix it up.

nateberkopec · 2017-08-29T18:23:12Z

👏 Give me a while to look at it, headed to Japan soon so not sure how much time I'll have. Last resort we can talk about it at Kaigi?

tenderlove · 2017-08-29T18:50:32Z

@nateberkopec sounds good. 😊

nateberkopec · 2017-08-29T18:53:43Z

I can understand if you don't want to be used for experimentation, but I would like to get this in to production systems sooner rather than later.

also I think I speak for the whole Puma team when I say that we have no problem with experimentation. hell Evan wanted to add native websockets and rewrite the entire reactor. so adding an optional feature...pretty tame 😆

nateberkopec

Couple of things just for "Puma style" concerns.

nateberkopec · 2017-08-29T23:24:52Z

lib/puma/server.rb

+          fast_write client, "\r\n".freeze
+        }
+      else
+        env[EARLY_HINTS] = lambda { |_| }


Why is this necessary? I guess I don't understand.

This is so that app code can run without changes even if the server is being run without early hints support. The example app that @eileencodes put in the PR description can be run whether early hints is enabled or not (it prevents the app code from writing a if env[EARLY_HINTS] conditional)

nateberkopec · 2017-08-29T23:25:28Z

lib/puma/server.rb

@@ -595,6 +600,24 @@ def handle_request(req, lines)
      env[RACK_INPUT] = body
      env[RACK_URL_SCHEME] =  env[HTTPS_KEY] ? HTTPS : HTTP

+      if @early_hints
+        env[EARLY_HINTS] = lambda { |links|


It would be nice to have this lambda live somewhere other than the body of this method.

Agree. @eileencodes and I were trying to move it out of here, but the lambda body needs to access the io object (client) and fast_write is a private method. I'd love to move the lambda out of here, but I am not sure how.

nateberkopec · 2017-08-29T23:27:49Z

lib/puma/dsl.rb

@@ -241,6 +241,10 @@ def tcp_mode!
      @options[:mode] = :tcp
    end

+    def early_hints!


I know our DSL style is really inconsistent, but for futureproofing I'd prefer:

def early_hints(answer=true) @options[:early_hints] = answer end

nateberkopec · 2017-08-29T23:29:04Z

lib/puma/server.rb

+          links.each do |link|
+            fast_write client, "Link: <#{link[:link]}>; rel=preload"
+            if as = link[:as]
+              fast_write client, "; as=#{as}"


Should we auto-detect this part? I did some of this in rack-http-preload..

Unsure. h2o pushes the resource regardless. I think this is something we need to iterate on. (IOW, I don't have an answer)

It's also possible we may not want to auto-detect as at the puma level. I just wanted to ask the question.

nateberkopec · 2017-08-29T23:32:01Z

lib/puma/server.rb

@@ -97,6 +98,10 @@ def tcp_mode!
      @mode = :tcp
    end

+    def early_hints!


I think you were copying tcp_mode!, but that's a ! method because it doesn't make sense for a server to be in tcp_mode and then not be in tcp mode later. Although I can't think of why anyone ever would (restarting and disabling early hints as the config changes?), it's more plausible that a server could toggle its support of early_hints. So this should probably just be treated like a straight-up accessor.

Moved to an attr_accessor and deleted this method.

nateberkopec · 2017-08-29T23:36:50Z

test/test_puma_server.rb

+
+    data = sock.read
+
+    assert_equal <<~eos.split("\n").join("\r\n") + "\r\n\r\n", data


Rubocop is barfing on your squiggly heredoc.

I saw the error, and I think RuboCop needs to be fixed. This is new syntax in Ruby 2.3. But also, since this is new syntax in Ruby 2.3, and you probably want to run the Puma tests on older versions of Ruby anyway, we'll fix it. 😉

Fixed the tests here too. 👍 I also added a test to test that early hints is off by default.

tenderlove · 2017-09-07T22:23:54Z

/cc @rafaelfranca

nateberkopec · 2017-09-22T20:43:32Z

Need to look at this one more time when I'm not jetlagged but I think this is ready.

tenderlove · 2017-09-28T19:25:01Z

Hey folks. I've been talking to @matthewd about this feature. He convinced me that the lambda should take a header hash just like the header hash from a rack response. This would alleviate webservers from knowing how the Link header is formatted, and may allow us to send other headers (like X- headers) for debugging purposes.

IOW, we should change the API to be something like this:

env[EARLY_HINTS].call(“Link”: [“<...>; rel=preload; as=..”, “..”])

WDYT?

nateberkopec · 2017-10-03T15:24:45Z

@tenderlove If I'm reading the HTTP 103 spec correctly:

A server MUST NOT include Content-Length, Transfer-Encoding, or any
hop-by-hop headers ([RFC7230], section 6.1) in the informational
response using the status code.
A client MAY speculatively evaluate the headers included in the
informational response while waiting for the final response. For
example, a client may recognize the link header of type preload and
start fetching the resource. However, the evaluation MUST NOT affect
how the final response is processed; the client must behave as if it
had not seen the informational response.

So, 103 status CANNOT contain a Connection, Content-Length or Transfer-Encoding header, but MAY contain any other header?

If I'm getting that right, your change makes sense. But should anyone be responsible for omitting the prohibited headers? Puma's job or someone elses?

tenderlove · 2017-10-03T16:41:13Z

If I'm getting that right, your change makes sense. But should anyone be responsible for omitting the prohibited headers? Puma's job or someone elses?

I think if you wanted to omit them in Puma it would be fine. However, I'd expect the proxy to complain or raise an error if it got those headers, so maybe Puma shouldn't care? It would be nice for the user to get an exception closer to where the error is occurring, but I don't think it's necessary.

This commit adds the early hints lambda to the headers hash. Users can call it to emit the early hints headers. For example: ``` class Server def call env if env["REQUEST_PATH"] == "/" env['rack.early_hints'].call("Link" => "</style.css>; rel=preload; as=style\n</script.js>; rel=preload") [200, { "X-Hello" => "World" }, ["Hello world!"]] else [200, { "X-Hello" => "World" }, ["NEAT!"]] end end end run Server.new ``` In this example, the server sends stylesheet and javascript early hints if the proxy supports it, it will send H2 pushes to the client. Of course not every proxy server supports early hints, so to enable the early hints feature with puma you have to pass the configuration variable, `--early-hints`. If `ENV['rack.early_hints']` is not set then early hints is not supported by the webserver. Early hints is off by default.

eileencodes · 2017-10-03T18:19:29Z

I've updated the PR so that now the lambda now takes a header hash and tested this in the Rails implementation for our test app. Let me know if you'd like other changes but I think this is good to go now.

PikachuEXE · 2018-03-23T08:19:43Z

lib/puma/server.rb

+                fast_write client, "#{k}: #{v}\r\n"
+              end
+            else
+              fast_write client, "#{k}: #{v}\r\n"


Should this be fast_write client, "#{k}: #{vs}\r\n" instead?
I don't see v defined in this level

v is defined right here https://github.com/puma/puma/pull/1403/files/ea37ada76937ff6b3988e952f598c26cf373904e#diff-ef4c610f0ce605eb90f2810856394fd5R604

Oh i see what you're saying - I will fix it

This is similar to puma/puma#1403

ioquatix · 2019-02-09T08:25:59Z

Was this ever integrated into the Rack SPEC? I had a look but couldn't see anything on the latest master branch.

What was the final design of the API?

I'm implementing this in falcon and support H2 server push directly so it would be fun to compare.

ioquatix · 2019-02-09T11:19:32Z

I implemented the design that was merged into puma.

socketry/falcon@248d0a4

Here is my brief feedback:

It's very HTTP/1 centric (i.e. using headers). For HTTP/2, I need to parse the headers and turn that into push requests. I guess it's okay. I don't know how I'd handle other headers correctly.
I only add the lambda to the rack env when it's possible to send early hints. So, it's conditionally enabled. Is that okay? Because I don't bother to support 103 Early Hints yet, but only support H2 push promises.
I wonder if exposing this in the application layer is really a good idea. I feel like a sufficiently intelligent server could figure out what things to send and when, and additionally cache what the client has/has not received.

In summary: is this interface too generic? Would it be better to limit it to push-promise functionality, or is it good to have it allow any header?

ioquatix · 2019-02-10T09:57:56Z

I wrote up a brief summary regarding implementing this feature.

https://www.codeotaku.com/journal/2019-02/falcon-early-hints/index

eileencodes force-pushed the early-hints branch from e2f4042 to 59764b6 Compare August 29, 2017 17:56

nateberkopec requested changes Aug 29, 2017

View reviewed changes

nateberkopec self-assigned this Aug 29, 2017

nateberkopec added the feature label Aug 29, 2017

eileencodes force-pushed the early-hints branch 2 times, most recently from 67b2347 to d444ed3 Compare September 20, 2017 16:39

nateberkopec approved these changes Sep 22, 2017

View reviewed changes

eileencodes changed the title ~~WIP: Add early hints feature~~ Add early hints feature Sep 23, 2017

eileencodes force-pushed the early-hints branch 2 times, most recently from c55bf5c to 4b87272 Compare September 26, 2017 17:58

eileencodes mentioned this pull request Sep 28, 2017

Implement H2 Early Hints for Rails rails/rails#30744

Merged

eileencodes force-pushed the early-hints branch from 4b87272 to ea37ada Compare October 3, 2017 17:22

nateberkopec merged commit 0169974 into puma:master Oct 4, 2017

dependabot-preview bot mentioned this pull request Mar 22, 2018

Bump puma from 3.10.0 to 3.11.3 sumyapp/basecamp3_slack_notify#29

Merged

PikachuEXE reviewed Mar 23, 2018

View reviewed changes

PikachuEXE mentioned this pull request Mar 23, 2018

H2 Early Hints cannot be used with response streaming rails/rails#32329

Closed

PikachuEXE added a commit to PikachuEXE/passenger that referenced this pull request Apr 4, 2018

Provide rack.early_hints in middleware

3d5d85a

This is similar to puma/puma#1403

CamJN pushed a commit to phusion/passenger that referenced this pull request May 7, 2018

Provide rack.early_hints in middleware

652af9f

This is similar to puma/puma#1403

evanleck mentioned this pull request Feb 21, 2019

HTTP 103 Early Hints ohler55/agoo#52

Closed

composerinteralia mentioned this pull request Oct 25, 2019

Use HTTP/2 early hints thoughtbot/suspenders#1006

Closed

eugeneius mentioned this pull request May 27, 2020

Add status the 103 Early Hints as Not content for action_dispatch rails/rails#39417

Merged

casperisfine mentioned this pull request Jul 28, 2020

Specify rack.provisional_response to send 1xx responses rack/rack#1701

Closed

sync-by-unito bot mentioned this pull request Apr 9, 2021

Bump puma from 3.9.1 to 3.12.6 yujiloh0816/sample_app#12

Closed

dentarg mentioned this pull request Aug 15, 2023

Expect continue socketry/falcon#204

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add early hints feature #1403

Add early hints feature #1403

eileencodes commented Aug 29, 2017 •

edited

tenderlove commented Aug 29, 2017

tenderlove commented Aug 29, 2017 •

edited

nateberkopec commented Aug 29, 2017

tenderlove commented Aug 29, 2017

nateberkopec commented Aug 29, 2017

nateberkopec left a comment

nateberkopec Aug 29, 2017

tenderlove Aug 30, 2017

nateberkopec Aug 29, 2017

tenderlove Aug 30, 2017

nateberkopec Aug 29, 2017

eileencodes Sep 20, 2017

nateberkopec Aug 29, 2017

tenderlove Aug 30, 2017

nateberkopec Aug 30, 2017

nateberkopec Aug 29, 2017

eileencodes Sep 20, 2017

nateberkopec Aug 29, 2017

tenderlove Aug 30, 2017

eileencodes Sep 20, 2017

tenderlove commented Sep 7, 2017

nateberkopec commented Sep 22, 2017

tenderlove commented Sep 28, 2017

nateberkopec commented Oct 3, 2017

tenderlove commented Oct 3, 2017

eileencodes commented Oct 3, 2017

PikachuEXE Mar 23, 2018

eileencodes Mar 28, 2018

eileencodes Mar 28, 2018

ioquatix commented Feb 9, 2019

ioquatix commented Feb 9, 2019 •

edited

ioquatix commented Feb 10, 2019


		data = sock.read

		assert_equal <<~eos.split("\n").join("\r\n") + "\r\n\r\n", data

Add early hints feature #1403

Add early hints feature #1403

Conversation

eileencodes commented Aug 29, 2017 • edited

tenderlove commented Aug 29, 2017

tenderlove commented Aug 29, 2017 • edited

nateberkopec commented Aug 29, 2017

tenderlove commented Aug 29, 2017

nateberkopec commented Aug 29, 2017

nateberkopec left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tenderlove commented Sep 7, 2017

nateberkopec commented Sep 22, 2017

tenderlove commented Sep 28, 2017

nateberkopec commented Oct 3, 2017

tenderlove commented Oct 3, 2017

eileencodes commented Oct 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ioquatix commented Feb 9, 2019

ioquatix commented Feb 9, 2019 • edited

ioquatix commented Feb 10, 2019

eileencodes commented Aug 29, 2017 •

edited

tenderlove commented Aug 29, 2017 •

edited

ioquatix commented Feb 9, 2019 •

edited