Skip to content

Commit

Permalink
fix(gateway): Don't cache 302 responses from the registry.
Browse files Browse the repository at this point in the history
This affects only the communication with the registry, not with anything
related to sending requests to the implementer's graph or its downstream
services.

A `HEAD` request has no body to cache and a 304 response could have only
been negotiated (with the server) by means of a conditional request header
(e.g., `if-none-match` or `if-modified-since`).

We do NOT want to write to the cache in either of these cases!  Without
avoiding this cache write, we will invalidate the cache, thus causing
subsequent conditional requests (e.g., `If-None-Match: "MD%") to be lacking
content to conditionally request against and necessitating a full
request/response.
  • Loading branch information
abernix committed Jun 29, 2020
1 parent e4341f7 commit 2f29c60
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 1 deletion.
1 change: 1 addition & 0 deletions packages/apollo-gateway/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
> The changes noted within this `vNEXT` section have not been released yet. New PRs and commits which introduce changes should include an entry in this `vNEXT` section as part of their development. When a release is being prepared, a new header will be (manually) created below and the appropriate changes within that release will be moved into the new section.
- The default branch of the repository has been changed to `main`. As this changed a number of references in the repository's `package.json` and `README.md` files (e.g., for badges, links, etc.), this necessitates a release to publish those changes to npm. [PR #4302](https://github.com/apollographql/apollo-server/pull/4302)
- The cache implementation for the HTTP-fetcher which is used when communicating with the Apollo Registry when the gateway is configured to use [managed federation](https://www.apollographql.com/docs/graph-manager/managed-federation/overview/) will no longer write to its cache when it receives a 304 response as such a response indicates that the cache used to conditionally make the request must already be present. [PR #TODO]()

## 0.16.9

Expand Down
13 changes: 12 additions & 1 deletion packages/apollo-gateway/src/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,18 @@ export class HttpRequestCache implements CacheManager {
}

async put(request: Request, response: Response) {
let body = await response.text();
// A `HEAD` request has no body to cache and a 304 response could have
// only been negotiated by using a cached body that was still valid.
// Therefore, we do NOT write to the cache in either of these cases.
// Without avoiding this, we will invalidate the cache, thus causing
// subsequent conditional requests (e.g., `If-None-Match: "MD%") to be
// lacking content to conditionally request against and necessitating
// a full request/response.
if (request.method === "HEAD" || response.status === 304) {
return response;
}

const body = await response.text();

this.cache.set(cacheKey(request), {
body,
Expand Down

0 comments on commit 2f29c60

Please sign in to comment.