apollo-server: apollo-server-plugin-response-cache: cache hint not working

My apollo graphql server setup uses the following:

    "apollo-datasource-rest": "^0.6.9",
    "apollo-server-express": "^2.9.12",
    "apollo-server-plugin-response-cache": "^0.3.8",

My schema:

type Query {
  types: [Type!]! @cacheControl(maxAge: 3600)
}

type Type {
  id: ID!
  label: String!
  active: Boolean!
}

My responseCachePlugin config:

responseCachePlugin({
    sessionId: (requestContext) => (requestContext.request.http.headers.get('sessionId') || null),
  })

Neither the static cache hint like the above nor the dynamic cache hint in the resolver work. i.e., they don’t cache the response from the datasource (REST API).

A bunch of other devs reported this issue on Spectrum. One of the Spectrum posts points to the problem:

The property requestContext.overallCachePolicy doesn't get updated by the cache hint, nither the static one in schema or the dynamic one ...
The value is always the cacheControl you configured when creating ApolloServer

github link to the line in the code where there is a potential problem: https://github.com/apollographql/apollo-server/blob/1acf62d903253f5669c0c4f7255c8a87b95b5e6c/packages/apollo-server-plugin-response-cache/src/ApolloServerPluginResponseCache.ts#L243

Spectrum post that details the above issue: https://spectrum.chat/apollo/apollo-server/apollo-server-plugin-response-cache-is-not-working-properly~e3390cd4-9e3f-4919-88b4-92508e235e38

Other Spectrum posts that talk about this problem:

  1. https://spectrum.chat/apollo/apollo-server/cachecontrol-does-not-work-unless-defaultmaxage-is-set~0e4dc3c5-7cdc-40be-b833-0b2d436d1288
  2. https://spectrum.chat/apollo/apollo-server/does-rediscache-work-in-dev-mode~18358268-8d11-4b13-a0c9-abddfb6623f3

cc to: @trevor-scheer

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Reactions: 37
  • Comments: 64 (17 by maintainers)

Commits related to this issue

Most upvoted comments

It’s disappointing the cache only works if you set a global caching settings:

  const server = new ApolloServer({
    cacheControl: {
       defaultMaxAge: 50,
    },
    plugins: [responseCachePlugin()],
    cache: new RedisCache({
      host: '127.0.0.1',
      namespace: 'apollo-cache:',
    }),

I am using merge-graphql-schemas with Redis and I am unable to get anything cached without the global settings.

I think it is a great idea to @cacheControl in the schema but it would be even better it it works first.

It is documented correctly but the result is not something usable. If you can’t set a overall cache, the cache doesn’t work at all. This is basically the issue. I wonder if someone from Apollo actively looks at the issues?

Applying this diff to your repo and running your curl command, the header is set to 100.

diff --git a/index.js b/index.js
index 4533fd6..eda16c5 100644
--- a/index.js
+++ b/index.js
@@ -1,7 +1,7 @@
 const { ApolloServer, gql } = require("apollo-server");
 
 const typeDefs = gql`
-  type Book { #@cacheControl(maxAge: 100)
+  type Book @cacheControl(maxAge: 100) {
     title: String
     author: String
   }
@@ -31,10 +31,6 @@ const resolvers = {
 const server = new ApolloServer({
   typeDefs,
   resolvers,
-  cacheControl: {
-    defaultMaxAge: 1000,
-  },
-  // cacheControl: true,
 });
 
 // The `listen` method launches a web server.

The point is that passing true for cacheControl enables a backwards-compatibility mode that you don’t want, but just leaving it out (or specifying an object with other options in it) gives the result you want.

Definitely a lesson here about the naming of boolean options: we should have been more thoughtful and named it includeCacheControlExtension or cacheControl: {includeExtension}.

This is a bug. The overall cache policy is documented correctly, yes, however the documentation is very specific that the default is that things are uncacheable:

By default, root fields (ie, fields on Query and Mutation) and fields returning object and interface types are considered to have a maxAge of 0 (ie, uncacheable) if they don’t have a static or dynamic cache hint.

The current behavior is that @cache-control doesn’t work, period, unless you also set a defaultMaxAge. That means you have to explicitly annotate every type you have with @cache-control and then set defaultMaxAge to something enormous, because of the overall cache policy logic.

I will say that the overall design approach here is fantastic! This seems like it should be an easy thing to fix frankly.

I tried without defaultMaxAge and caching is working for me. I noticed that if we are not using cache control hints properly then it looks like it is not working. Make sure that cache hint is applied to a field and its nested non scalar fields as well then only caching will work. Otherwise when it finds nested non scalar field is missing cache control so it tries to use defaultMaxAge and which is 0 by default. Hence, it looks like caching is not working.

Some points to be noted

  • Resolver level caching policy takes higher precedence than schema-object level
  • Lowest cache expiration takes highest precedence
  • Responses containing GraphQL errors or no data are never cached

Facing similar issue, when I use caching at the Apollo server level then everything is cached I can see no DB queries in log after the first request. Caching seems to be working

 cacheControl: {
    defaultMaxAge: 60
},

But when I try the fine grain caching at resolvers I can see the maxAge set in HTTP response but the response doesn’t seem to be cached as I can see DB queries on each request

const resolvers = {
  Query: {
    post: (_, { id }, _, info) => {
      info.cacheControl.setCacheHint({ maxAge: 60, scope: 'PUBLIC' });
      return find(posts, { id });// some db call
    }
  }
}

Am I doing anything wrong here?

Then queries against someField will be cached for 50, queries against someOtherField will be cached for 5, and queries that hit both fields will also be cached for 5 because they contain some low-TTL data. Is that not what you’re experiencing?

It is how it should work but now someField query is cached also for 5 sec - not for 50 sec. Just tested it.


Btw it would be also great to have ability to set default scope on all queries to PRIVATE. In my app they all should be ‘PRIVATE’ so I need to mark each individually to be ‘PRIVATE’ instead of setting it once eg like this:

cacheControl: {
  defaultMaxAge: 5,
  defaultScope: 'PRIVATE'
}

@glasser Thanks for the reply. I wasn’t testing field level caching, so I’m not sure on that. I will try to test it later in my schema and let you know the result. My object example would be like this:

type User @cache-control(maxAge:50)  {
  someField: Foo
  someOtherField: Bar
}

In the above case, with defaultMaxAge=5, if you fetch User by id, it will be cached for 5 seconds, not 50.

the ability to mark an object/interface field as “inherit parent maxAge” just like scalar fields do.

Really I just want it to not use defaultMaxAge, at all, if a cache hint has been set, either via @cache-control or dynamically. To me default means default, i.e. use this if there is no other value. Instead the way it works in my testing is, override cache hint value with defaultMaxAge if it is less than what is currently computed, i.e. defaultMaxAge is the “uber parent” of the object graph. I think that’s what is throwing people for a loop here.

And sorry yes let me try to get you an example git repo later this week, I know this is difficult without seeing a concrete thing that doesn’t work as expected. 😃

In case you want to save the effort of setting up Redis, getting the config working etc, you may wanna have a look at https://graphcdn.io. I’m building that right now, especially to make caching easier, as I struggled with it myself before.

Absolutely, I couldn’t agree more. It’s designed perfectly and we hope it gets fixed. I’ll send this thread to the Apollo support team in several channels. Hopefully we get an answer soon.

I also noticed that only the first query has the right cacheHint:

  • i define a defaultMaxAge of 3600 (without it, it does not work at all)
  • on a field i add a cachehint of 60 (1min)
  • i run the query twice and inspect both the headers and whether my db request is performed)

first request responses with cache-control: max-age=60, public second request responses with cache-control: max-age=3600, public

it seems like apollo server caches the cache hints as well!

Edit:

its clear. Not setting defaultMaxAge --> caching DOES NOT WORK AT ALL you have to set defaultMaxAge to something huge, because it defines the maximum caching time, not the default caching time, when no other cache hint is available.

But setting cacheHints dynamically for every resolver also does not work properly. As mentioned above it only works shortly. After further investigation it seems that it works for the given cachehint time, after that it falls back to the global setting.

Something is totally wrong in the cache logic and its not only the flawed design of defaultMaxAge

Edit2: its really confusing to debug.

But it seems clear that resolver-level cache hints get lost when it tries to use the cache.

Edit3: it had something to do how we set cachehints dynamically, i think i solved it.

But the one flaw remains: defaultMaxAge is the maximum TTL that you can set. if you omit it, no resolver will get cached

After reading through parts of this thread I’m still pretty confused, and I can’t tell if Apollo Server is not working at it should; or else if I just don’t understand what it’s supposed to do.

To clarify, if I add this at the top of a resolver:

cacheControl.setCacheHint({ maxAge: 300 });
console.log('hello world!');

I would expect subsequent queries to that resolver to not execute the console.log for the next five minutes? If so then that’s not what I’m seeing currently.

To follow up on my last post, I only just now finally understood that this is about setting cache hints so that Apollo can figure out whether the entire request is cacheable or not; and not telling Apollo to cache individual fields.

In other words, if I have 9 cached fields and 1 uncacheable fields in my request I falsely expected after reading the documentation that those 9 fields would be cached and not trigger database, API, etc. requests. Whereas the truth (as I now understand it, I might still be wrong?) is that none of my 10 fields will be cached because the entire request is not cacheable.

On the other hand, if I used Datasources for those 9 fields then those database, API, etc. requests would be cached independently of whether the full GraphQL can be cached or not.

Again sorry if I’m wrong and just making things more confusing, but I suspect other people might’ve misunderstood this as well.

I’m seeing exactly this problem also. I was working on adding response caching today and couldn’t figure out why I wasn’t seeing any caching happening. I’m glad I’m not going mad but also disappointed that response caching is apparently broken 😦

Here’s an implementation of something that’s similar to the inherit concept I discussed above: https://github.com/apollographql/apollo-server/pull/5247 I’m interested in feedback from folks who were interested in this issue!

I only want to be able to cache things that have explicit cache hints, without defining the defaultMaxAge. cache hints i define in resolvers don’t work without defaultMaxAge, and when I define it, apollo server automatically caches things I don’t want it to cache (even without hints) which results in unexpected behaviors on the front end.

How do I only cache things that have explicit hints, and disable automatic root object caching? It is not possible to do that?

@glasser I might be wrong but I get the impression this is maybe more an issue with unclear documentation than an actual bug? I haven’t had time to revisit this issue but now I just learned about datasources and I feel like that’s one more caching parameter I need to juggle with… I think a comprehensive blog post or example app that clearly outlines the different caching methods available with Apollo Server and the pros and cons of each one would be helpful here.

Anyone figure it out how to make cache control works? Im also with the same situation

This metadata is an input into several cache-related features, like the calculation of cache-control HTTP headers on responses and the full response cache plugin, but is not itself a cache.

Oh, does this mean that setting the metadata by itself should not be expected to actually cache the data? If so that would explain why I didn’t notice any difference in my implementation. If that’s the case maybe the docs could state this explicitly, it might be confusing for others as well?

I think that I have the same issue with the cache-control header not set when defaultMaxAge is not defined. @glasser here is a repo with a detailed reproduction recipe. Let me know if something is not clear.

https://github.com/mthenw/apollo-cache-issue

@glasser I’m not sure exactly what you need for a self-contained reproduction, but I feel my initial comment on expected vs actual behaviour describes the issue most here are experiencing:

My initial expectation when testing this kind of setup:

cacheControl: {
    defaultMaxAge: 60
},

type Destination {
    name: String! @cacheControl(maxAge: 10)
    distance: Float!
}

would be for Destination.distance to cache for 60 seconds, and Destination.name to cache for 10. But the result is that the entire Destination type caches for the minimum maxAge across all fields, in this case 10.

If I can offer anything further that might help, please let me know.

@Jylth agree that the logic is not what you’d expect, but is technically documented correctly (but could probably be improved):

Apollo Server then combines these hints into an overall cache policy for the response. The maxAge of this policy is the minimum maxAge across all fields in your request.

My initial expectation when testing this kind of setup:

cacheControl: {
    defaultMaxAge: 60
},

type Destination {
    name: String! @cacheControl(maxAge: 10)
    distance: Float!
}

would be for Destination.distance to cache for 60 seconds, and Destination.name to cache for 10. But the result is that the entire Destination type caches for the minimum maxAge across all fields, in this case 10.

I tried without defaultMaxAge and caching is working for me. I noticed that if we are not using cache control hints properly then it looks like it is not working. Make sure that cache hint is applied to a field and its nested non scalar fields as well then only caching will work. Otherwise when it finds nested non scalar field is missing cache control so it tries to use defaultMaxAge and which is 0 by default. Hence, it looks like caching is not working.

Some points to be noted

  • Resolver level caching policy takes higher precedence than schema-object level
  • Lowest cache expiration takes highest precedence
  • Responses containing GraphQL errors or no data are never cached

HUGE THANKS ankuradhey!!! Your comment lightened up my implementation! So… if we want defaultMaxAge: 0, than we need to explicitly define the ‘cacheControl(maxAge: XXX)’ directive to all non scalar fields of the respected query or mutation. Once I did that, the all the fields marked with the directive started to respect the the defined maxAge and all the other fields (non marked) remained with 0 caching time.

Hi @SachaG !

As mentioned above it is very challenging for me to provide support on this feature just based on single line snippets, rather than complete examples.

apollo-server-plugin-response-cache is a full response cache that caches full GraphQL responses if every field queried in it is cacheable. So if a query asks for a cacheable field and an uncacheable field, it will re-run the resolver for the cacheable field too because the whole response is not cacheable.

Much of the confusion in this PR is about what the definition of a field being marked as cacheable is, because the default behavior (while internally consistent and accurately documented, AFAIK) is surprising and perhaps not ideal.

@mthenw: According to the docs at https://www.apollographql.com/docs/apollo-server/api/apollo-server/

If set to true, adds tracing or cacheControl metadata to the GraphQL response. This is primarily intended for use with the deprecated Engine proxy. cacheControl can also be set to an object to specify arguments to the apollo-cache-control package, including defaultMaxAge, calculateHttpHeaders, and stripFormattedExtensions.

As documented, cacheControl: true literally just includes this somewhat useless metadata in the GraphQL response. This was something we implemented before we implemented writing caching HTTP response headers in apollo-server, so we wanted to keep its behavior consistent even as we changed the default for users who don’t specify cacheControl at all. (Additionally, this value was intended for folks running behind the old binary engineproxy, which I believe calculated its own HTTP response headers based on this metadata, so having two separate sets of calculation seemed like a bad plan.)

The logic in the code is:

    if (this.config.cacheControl !== false) {
      let cacheControlOptions: CacheControlExtensionOptions = {};
      if (
        typeof this.config.cacheControl === 'boolean' &&
        this.config.cacheControl === true
      ) {
        // cacheControl: true means that the user needs the cache-control
        // extensions. This means we are running the proxy, so we should not
        // strip out the cache control extension and not add cache-control headers
        cacheControlOptions = {
          stripFormattedExtensions: false,
          calculateHttpHeaders: false,
          defaultMaxAge: 0,
        };
      } else {
        // Default behavior is to run default header calculation and return
        // no cacheControl extensions
        cacheControlOptions = {
          stripFormattedExtensions: true,
          calculateHttpHeaders: true,
          defaultMaxAge: 0,
          ...this.config.cacheControl,
        };
      }

Are there docs somewhere that encouraged you to set cacheControl: true that we should fix?

My solution (for now) was to make custom @CacheControl decorator (with type-graphql):

import { UseMiddleware } from 'type-graphql';

export function CacheControl(hint: CacheHint) {
  return UseMiddleware(({ context, info }, next) => {
    console.log('Called CacheControl');

    context.cacheOptions = {
        ttl: hint.maxAge,
    };
  
  // replace with setCacheHint when it is working properly again
  // info.cacheControl.setCacheHint(hint);
    return next();
  });
}

@CacheControl({ maxAge: 60 })

Obviously scope: private/public is missing from context.cacheOptions but many times setting ttl is enough.

Tested only with type-graphql -module

That is not my understanding of how defaultMaxAge works (though of course, there may be a bug, that’s why we’re here).

If you set defaultMaxAge to 5 but have a schema like

type Query  {
  someField: Foo @cacheControl(maxAge: 50)
  someOtherField: Bar
}

Then queries against someField will be cached for 50, queries against someOtherField will be cached for 5, and queries that hit both fields will also be cached for 5 because they contain some low-TTL data. Is that not what you’re experiencing?

The main feature that I think is missing here (other than features that would make this model more comprehensible/inspectable) is the ability to mark an object/interface field as “inherit parent maxAge” just like scalar fields do.

But I don’t understand how your example differs from your example without setting defaultMaxAge (unless this is what helps us narrow down what the bug is!). I am really looking forward to the clear example of this bug given by somebody on this thread!

@jbarefoot How is that better than setting defaultMaxAge to 60?

@glasser That’s much appreciated, I’ll try to find some time soon to create a basic repo that illustrates the issue. In the meantime, I implemented a workaround which I’ll post here for others. The workaround is to use a wrapper function around every resolver function, that checks for a cache hint and adds it if it doesn’t exist. As long as you remember to use the wrapper on every query resolver, it works fine. Then you set defaultMaxAge to something really high, like a day, so it will never be the minimum in the overall computed cache policy.

const defaultExpiry = 60  // in seconds
const defaultCacheHint = { maxAge: defaultExpiry, scope: 'PUBLIC' };

async function resolverWrapper(info, func) {
  console.log("Cache hint already set to:", info.cacheControl.cacheHint)

  // The hack here is that if cache-control has NOT been set for a type/field/whatever, then you will get { maxAge: 0 } for info.cacheControl.cacheHint i.e. uncached
  // If cache-control HAS been set, then you get { maxAge: <someInt>,  scope: undefined }, i.e. there will be a "scope" key
  // So only set default expiry/TTL if the "scope" key does NOT exist. (Checking for whether the cacheHint is defined is just being defensive)
  if(!info.cacheControl.cacheHint || !("scope" in info.cacheControl.cacheHint)) {
    console.log("Cache hint was NOT set so defaulting to:", defaultCacheHint)
    info.cacheControl.setCacheHint(defaultCacheHint);
  }
  return func()
}

// Use it from a resolver like this:
loadSomeThings: async (_, {fromDate, toDate}, __, info) => resolverWrapper(info, async () => {
  const sql = `select foo, bar, baz, start_date from some_table where start_date between '${fromDate}' and '${toDate}'`
  return await runMySQLQuery( sql )
}),