Apollo-server: maxAge of 0 results in no cache-control header

[rainboxx]

Problem

We're facing issues with persisted queries using GET requests and IE11. This lovely browser is serving responses without a cache-control header from its cache. While this is ok for most of _our current_ application's queries, some queries should not be cached.

We consider leaving out cache-control headers for a maxAge setting of 0 a bug. Also the scope is ignored while Cache-Control: private would be a totally reasonable header.

Workaround

A workaround for us is to annotate specific types with a maxAge of 1 second, which is ugly but works for these kind of requests (we don't expect the user to be faster, but in case he is, it results in glitches).

Furthermore, this workaround for types containing other types is not that easy: as soon as another type without a cache hint is added to a type, the cache-control header is no longer sent to the browser because the default maxAge is 0 and therefore lower than 1. We need to annotate all types used in a type as well, which results in a mess: some types by itself might benefit from a different cacheControl settings in certain situations.

This code is responsible for leaving out the headers for situations when the lowest maxAge is 0: https://github.com/apollographql/apollo-server/blob/821578775d3ca62ac33a76a8d3d8f51f412cdd80/packages/apollo-cache-control/src/index.ts#L171-L178

Possible solution

A possible solution for our case would be to still send headers in case the lowest maxAge is 0.

I'd be happy to submit a PR for a proper solution that enables consumers of apollo-server to specify a maxAge of 0 or scope only if this is a reasonable approach.

However, it seems more cache requirements are out there (eg. the old and long untouched #1295 as well as #1424, which might also work for our situation) so this solution might not fit to the overall roadmap. Unfortunately for us #2360 doesn't mention any plans for cacheControl.

[itsnotrisky]

I didn't encounter the issue mentioned, but the same code block might cause this issue.

Basically if you set maxAge from @cacheControl it will return undefined because lowestMaxAge will always looking at lowest value therefore no caching will be performed.

Unless your maxAge > cacheControl.defaultMaxAge then maxAge will be returned with lowest value but all your query will be cached.

[molomby]

This is a pretty serious bug, no?

Specifying Cache-Control: private, max-age: 0 is completely valid -- it tells shared caches (like CDNs) to exclude the resource, allows private caches (like the browser) to store the resource but instructs them to validate its freshness before using it. This is _very_ different than not returning a cache control header at all which, I believe, is the current behaviour.

At best this incorrectly conflates these two directives, causing sub-optimal caching, at worst it allows information (that has been explicitly marked as private) to be leaked/shared between requests from different users.

If i'm understanding correctly, due to the way cache hints from multiple queries are combine (by min'ing the max ages), it's possible that _adding_ a field to a query can cause the cache control header to be dropped, even if all the existing fields in the query are private.

Eg. with this type:

type User {
  id: ID
  name: String
  email: String @cacheControl(scope: PRIVATE)
  notes: String @cacheControl(scope: PRIVATE, maxAge: 0)
}

This query will return with a cache control header of Cache-Control: private:

query {
  User (id: "123") { id name email }
}

But if you also hit the notes field, the max age of 0 takes precedence and no cache control header is added:

query {
  User (id: "123") { id name email notes }
}

This implicitly makes the entire result cacheable, even though it's pretty clear the developer intended the opposite.

[timsuchanek]

In case you don't want to wait for a fix, here a shameless plug:
I created a dedicated GraphQL CDN where this is fixed - https://graphcdn.io
We send private, no-store in these cases to make it explicit.