fix: Commit request prior to cache lookup when using guest caching - #1536
fix: Commit request prior to cache lookup when using guest caching#1536TartanLlama wants to merge 6 commits into
Conversation
|
@cceckman-at-fastly do you think you could have a look at this and tell me if this is the right place to make the hostcalls necessary to update headers when using guest caching? |
TartanLlama
changed the title
|
Fixes #1178 |
| bool is_cacheable = false; | ||
| { | ||
| auto request_handle = Request::request_handle(request); | ||
| auto res = request_handle.is_cacheable(); |
There was a problem hiding this comment.
Assuming fetch_send_body above flushes the headers for the non-guest-caching cases:
I think we need to make sure the host & guest are in sync as of this call, which should map to the is_request_cacheable hostcall. There are some things the RFC says about how the Authorization header affects cacheability ("it's complicated" but we want to be sure we have the data here) -- and if they have changed the method, that plays in to it as well.
That touches on my other thought: is commit_headers just headers, or is it also method & URI? We should make sure those are flushed as well
2 participants
Currently, headers are not committed prior to
transaction_lookupcalls when using guest caching. As a result, if any headers are set that are marked as part of aVaryheader on a cached origin response, the cache lookup does not take them into account. This PR ensures that headers are committed in time.