early hint functionality #1996
early hint functionality #1996
Conversation
There was a problem hiding this comment.
The hints map[string]string prevents me from generating for example these headers:
Link: <https://fonts.google.com>; rel=preconnect
Link: </main.css>; rel=preload; as=style
Link: </common.js>; rel=preload; as=script
Link: </experimental.3eab3290.css>; rel=preload; as=style
Can you also add a test case that shows the Early Hints response and makes sure the normal response after it also works fine.
ctx.EarlyHints(map[string]string{
"https://fonts.google.com": "rel=preconnect",
"/main.css": "rel=preload; as=style",
"/common.js": "rel=preload; as=script",
"/experimental.3eab3290.css": "rel=preload; as=style", // <===== You wouldn't normally early hint this one because it's not stable
}) |
|
I'm wondering if this is the best API. Another way might be that this function has no arguments and just sends the headers set in the response already and then resets them. Not sure if an Early Hints response can also have other headers? |
The point of early hints is that the Link Headers are sent before the processing of the full response. Also the early hint links are not necessarily the same as the final response links. It is usually a smaller subset (i.e. link resources that are stable such as perhaps overall sites css c.f a particular page's css that developer's may change regularly). If the function didn't take arguments as to what the early hints are, I would need to waste time iterating over the currently set headers to determine which ones are eligible as valid early hints. This is also similar to the API of node.js: https://github.com/nodejs/node/pull/44180/files (except I made it into a map instead of a slice) |
|
@erikdubbelboer Is there a sample test that I can use as a template that tests something similar? |
|
The API I was thinking of is just: ctx.Response.Header.Add("Link", "...")
ctx.EarlyHints()Which then only send the Link header and not any other header that might already have been set. This design is more in line with what we already have and makes it easier to have no memory allocations. For an example of a test you can take the 100 continue test: Line 1881 in 48f3a2f |
But I still have to iterate over the headers to determine if the header is a There is no guarantee the developer will only add Links to header before calling |
|
The amount of headers will be so low that its probably around the same speed as the map. A map makes it really hard not to generate garbage which would make that much slower. |
|
@erikdubbelboer Done |
|
@erikdubbelboer Done |
There was a problem hiding this comment.
Sorry, 2 more changes.
- Can you make the function return an error so all the
nolint:errcheckcan go away and can instead be turned into error returns. Also check the error returned by Flush anddeferthereleaseWriterso it also gets released on error. - Can you add a simple example to the documentation of the function.
|
Tests are flaky. |
|
Thanks! I'll tag a release tomorrow. |
|
What's the difference between c.Conn() and using that compared to acquire/release? |
|
Conn() returns the raw connection. Writing directly to it could cause each Write() to be a separate TCP packet. The writer is buffered and will send everything in a single packet on Flush(). |
Draft of Early Hints functionality.
Fixes: #1992