nextjs - 💡(How to fix) Fix Dynamic Headers lead to Useless Cache [1 comments, 2 participants]

Issue

Our team recently introduced annotating all outbound requests (i.e. fetches) with a Correlation ID and have discovered that this breaks cache deduplication, effectively rendering it useless.

If we have multiple pods of our Next application and a user makes a request to /page1 which makes a fetch, the outbound request will have a Request header with correlation ID 123 and the result will be stored in cache (in our instances its shared cache in Redis, but it also applies to a singleton process with in memory cache). If another user, 2 seconds later, makes a request to /page1 Next will check to see if there is a result in cache before fetching from origin, but the correlation ID on that request might be 456 which means the cache key for these two requests are different and thus the fetch cache is rendered useless.

I found that the reason for this is that ALL headers are considered when generating the cache key per https://github.com/vercel/next.js/blob/canary/packages/next/src/server/lib/incremental-cache/index.ts#L373-L392.

// w3c trace context headers can break request caching and deduplication
// so we remove them from the cache key
if ('traceparent' in headers) delete headers['traceparent']
if ('tracestate' in headers) delete headers['tracestate']

const cacheString = JSON.stringify([
  MAIN_KEY_PREFIX,
  this.fetchCacheKeyPrefix || '',
  url,
  init.method,
  headers,
  init.mode,
  init.redirect,
  init.credentials,
  init.referrer,
  init.referrerPolicy,
  init.integrity,
  init.cache,
  bodyChunks,
])

There are some recent commits that exclude traceparent and tracestate from consideration of the cache key and I want to take this a step further and allow users to define a set of headers that will be excluded. The reason for this is that there are MANY implementations of this type of trace ID / correlation ID header outside of the ones in w3c so let users decide which ones they feel are dynamic and wouldn't make two fetches be considered different. Another example might be requests that send along a Date header, those might not want to be considered 'different' in the cache, so users can specify that in some list of excluded headers.

Proposal

Yes, I am interested in contributing.

We would need to add this cacheKeyExcludedHeaders in next.config.js that accepts an array of strings and then in the incremental-cache.ts file we could update the code to do this before defining cacheString.

  if (cacheKeyExcludedHeaders)) {
    cacheKeyExcludedHeaders.forEach((excludedHeader) => {
      if (excludedHeader in headers) {
        delete headers[excludedHeader];
      }
    });
  }

Another idea, could be to extend the patched fetch API to allow for a user defined cache key function that has the same signature as-is today

  async generateCacheKey(
    url: string,
    init: RequestInit | Request = {}
  ): Promise<string>

Originally posted by @dylandignan in https://github.com/vercel/next.js/discussions/76875