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:

apollo-server/packages/apollo-cache-control/src/index.ts

Lines 171 to 178 in 8215787

	// If maxAge is 0, then we consider it uncacheable so it doesn't matter what
	// the scope was.
	return lowestMaxAge
	? {
	maxAge: lowestMaxAge,
	scope,
	}
	: undefined;

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.

[liamchilds]

I came here with the same problem and while there is still no fix I thought I would share my "workaround" that can be adapted. The following works as an apollo plugin which sets the cache control header if it hasn't already been set.

export class CacheControlFallbackPlugin<TContext> implements ApolloServerPlugin<TContext> {
  public requestDidStart(): GraphQLRequestListener<TContext> {
    return {
      willSendResponse(requestContext: GraphQLRequestContextWillSendResponse<TContext>): ValueOrPromise<void> {
        const existingCacheHeader = requestContext.response.http?.headers.get('Cache-Control');
        if (!existingCacheHeader) {
          // This can be tailored....
          requestContext.response.http?.headers.set('Cache-Control', 'no-store, max-age=0');
        }
      },
    };
  }
}

[kdybicz]

I can confirm @liamchilds approach is working, I'm using exactly the same in my project.

[glasser]

The "workaround" seems reasonable. The approach Apollo Server's cache plugin has always taken was to only send cache headers for cacheable responses, on the assumption that clients wouldn't choose to cache responses without explicit header noting it's OK. If that assumption is wrong, fixing it with your own plugin seems reasonable. Alternatively, instead of looking at the existing header, you can just look at the request's cache policy and see if it's cacheable. The API for this is changing a bit in Apollo Server 3, where it'll be:

export class CacheControlFallbackPlugin<TContext> implements ApolloServerPlugin<TContext> {
  public async requestDidStart(): Promise<GraphQLRequestListener<TContext>> {
    return {
      async willSendResponse(requestContext: GraphQLRequestContextWillSendResponse<TContext>): Promise<void> {
        if (!requestContext.overallCachePolicy.policyIfCacheable()) {
          // This can be tailored....
          requestContext.response.http?.headers.set('Cache-Control', 'no-store, max-age=0');
        }
      },
    };
  }
}

In general my philosophy about the cache control plugin is that the main thing it does is calculate requestContext.overallCachePolicy; then, as a convenience, it also by default sets a Cache-Control header. But rather than providing lots of knobs for configuring exactly what that header looks like, if you don't like the default behavior, you can disable it (calculateHttpHeaders: false) and have your own plugin look at overallCachePolicy and set headers yourself.

I think this is working as intended.

[rainboxx]

It seems the docs have been rewritten 5 months ago to indicate that no header is sent if maxAge is zero. This technically solves this issue.

However, I still find this behaviour weird and misleading. The resulting reduced DX is pointed out by disclaimers in the documents, e.g. this one:

apollo-server/docs/source/performance/caching.md

Line 47 in 1de9af0

> **Important:** Apollo Server assigns each GraphQL response a `maxAge` equal to the _lowest_ `maxAge` among included fields. If any field has a `maxAge` of `0`, the response will not be cached at all.

The use of the cache plugin is not very straight-forward in this respect.

[glasser]

In an ideal world, how would Apollo Server know when to send a max-age=0 header and when to send no header at all? (That's the difference you're describing, right?)

[rainboxx]

I would probably see maxAge being undefined/null as default and indicating that no cache headers should be sent. Setting maxAge to a specific positive integer value would mean to cache the resource for the value given.

The point is that a specific value (may it be 0 or 3600) usually means something specific. In this regard only undefined is not specific, null could be treated the same.

[thehideki81]

I needed to be able to explicitly add max-age: 0, and not send header s if not set.

For this i used a decorator, and a custom plugin. The decorator sets a context value, that the plugin checks it and does what is needed.

The plugin is added to apollo-server plugins array.

Note: Used typescript.

Decorator:

import { CacheHint } from 'apollo-server-types'
import { UseMiddleware } from 'type-graphql'

export function CacheControl(hint: CacheHint) {
  return UseMiddleware(({ info, context }, next) => {
    info.cacheControl.setCacheHint(hint)
    // So we can control if we want to set explicit max-age zero headers later.
    context.cacheControlSet = true
    return next()
  })
}

Plugin:

export const cacheControlFallbackPlugin = (): ApolloServerPlugin => ({
  async requestDidStart(
    _request: GraphQLRequestContext
  ): Promise<GraphQLRequestListener> {
    return {
      willSendResponse(requestContext: GraphQLRequestContext): Promise<void> {
        console.log(requestContext.context)
        // Only if we have explicitly set max age to zero with CacheControl()
        if (
          requestContext.overallCachePolicy.maxAge === 0 &&
          requestContext.context?.cacheControlSet === true
        ) {
          requestContext.response.http?.headers.set(
            'Cache-Control',
            'max-age=0, no-store'
          )
        }
        return
      }
    }
  }
})

Usage, resolver:

  @Query(() => SomeDto)
  @CacheControl({ maxAge: 0 })
  async resolverFunction(
    @Arg('id', () => Int) id: number
  ): Promise<SomeDto> {
    return doStuff(id)
  }

[molomby]

This is still so broken. GET requests are cacheable by default:

The response to a GET request is cacheable; a cache MAY use it to satisfy subsequent GET and HEAD requests unless otherwise indicated by the Cache-Control header field
– RFC 9110 HTTP Semantics

HTTP is designed to cache as much as possible, so even if no Cache-Control is given, responses will get stored and reused if certain conditions are met.
– MDN HTTP caching: Heuristic caching

Removing the Cache-Control header when max-age=0 is the opposite of what should happen. This is especially true when the Cache-Control header includes additional directives like specifying a private scope, as per my earlier example. This can easily create a situation where data that has been explicitly marked as private by the developer has that designation removed and replaced with implicit permission to save the response in a shared cache.

Documenting the broken behaviour doesn't make the behaviour any less broken.

[glasser]

I don't really disagree with you. That said, a bit of a challenge here is that there does appear to be multiple desired outcomes here. I can see an argument for "as long as you have the (installed-by-default) cache control plugin installed (and haven't disabled HTTP header calculation), we should always set a cache-control header". But I also see @thehideki81 wants to only set max-age=0 if a field that explicitly uses CacheControl is invoked. So improving this for your use can might not be right for @thehideki81 's use case. And it does seem that the "always send a cache-control header" use case can be addressed by just setting calculateHttpHeaders: false and setting HTTP headers in your own plugin.

That said, you do have a point that HTTP RFCs encourage everyone to set cache-control on every response and that browsers will cache many GET requests by default. It's admittedly quite late in the AS4 release cycle (release candidate is out) but changing the behavior of calculateHttpHeaders: true (the default) to always set cache-control does seem worth considering.

[glasser]

@molomby How would you feel about changing the behavior of calculateHttpHeaders: true to use the same logic as we have now to decide whether to set a cache-control header based on the calculated policy (ie: policy must have positive max-age, response must have no errors) but in any case where that's not true, setting a fixed cache-control: no-store header? (and not caring if private was selected or not.)

[glasser]

(We could also keep the current semantics for POST and only add the new header for GET.)

[glasser]

OK, in AS4 we will set cache-control: no-store on any response that doesn't otherwise set cache-control, unless you disable the cache control plugin entirely, pass calculateHttpHeaders: false to it, or keep the AS3 behavior with calculateHttpHeaders: 'if-cacheable'. It's in @apollo/server@4.0.0-rc.16.