Skip to content

Commit

Permalink
fix incorrect fetch cache handling (vercel#58460)
Browse files Browse the repository at this point in the history
### What?
When `FetchCache` is used, cache gets were incorrectly bailing. This would result in unexpected behavior, like continuously revalidating a cache key, as described in vercel#57978.

### Why?
vercel#57902 introduced a refactor to the `FileSystemCache` and changed the interface of `get`, but this change was not propagated to `FetchCache`. Specifically, `fetchCache` was removed in favor of a new type `kindHint`. As a result, cache reads would always short circuit because `fetchCache` would never be defined.

### How?
This updates the interface on `FetchCache` to match what is defined on the base `CacheHandler`. I've also updated the args to both `get` and `set` to be derived from `CacheHandler` so we don't have any type inconsistencies in the future.

I will be following up with a test in the CLI repo to test against a deployed app (since minimalMode cannot be easily mocked in our test suite). Manually verified these changes against the repro in the original issue below, at the following URLs:

https://revalidate-vercel-test-iota.vercel.app/fetch-cache-test
https://revalidate-vercel-test-iota.vercel.app/revalidate-tag-test

Fixes vercel#57978
Fixes vercel#58306
  • Loading branch information
ztanner authored Nov 15, 2023
1 parent 375d85d commit 4cbfe7d
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 50 deletions.
36 changes: 9 additions & 27 deletions packages/next/src/server/lib/incremental-cache/fetch-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,19 +142,13 @@ export default class FetchCache implements CacheHandler {
}
}

public async get(
key: string,
ctx: {
tags?: string[]
softTags?: string[]
fetchCache?: boolean
fetchUrl?: string
fetchIdx?: number
}
) {
const { tags, softTags, fetchCache, fetchIdx, fetchUrl } = ctx
public async get(...args: Parameters<CacheHandler['get']>) {
const [key, ctx = {}] = args
const { tags, softTags, kindHint, fetchIdx, fetchUrl } = ctx

if (!fetchCache) return null
if (kindHint !== 'fetch') {
return null
}

if (Date.now() < rateLimitedUntil) {
if (this.debug) {
Expand Down Expand Up @@ -262,21 +256,9 @@ export default class FetchCache implements CacheHandler {
return data || null
}

public async set(
key: string,
data: CacheHandlerValue['value'],
{
fetchCache,
fetchIdx,
fetchUrl,
tags,
}: {
tags?: string[]
fetchCache?: boolean
fetchUrl?: string
fetchIdx?: number
}
) {
public async set(...args: Parameters<CacheHandler['set']>) {
const [key, data, ctx] = args
const { fetchCache, fetchIdx, fetchUrl, tags } = ctx
if (!fetchCache) return

if (Date.now() < rateLimitedUntil) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
import type { RouteMetadata } from '../../../export/routes/types'
import type { CacheHandler, CacheHandlerContext, CacheHandlerValue } from './'
import type { CacheFs } from '../../../shared/lib/utils'
import type {
CachedFetchValue,
IncrementalCacheKindHint,
} from '../../response-cache'
import type { CachedFetchValue } from '../../response-cache'

import LRUCache from 'next/dist/compiled/lru-cache'
import path from '../../../shared/lib/isomorphic/path'
Expand Down Expand Up @@ -120,18 +117,9 @@ export default class FileSystemCache implements CacheHandler {
}
}

public async get(
key: string,
{
tags,
softTags,
kindHint,
}: {
tags?: string[]
softTags?: string[]
kindHint?: IncrementalCacheKindHint
} = {}
) {
public async get(...args: Parameters<CacheHandler['get']>) {
const [key, ctx = {}] = args
const { tags, softTags, kindHint } = ctx
let data = memoryCache?.get(key)

// let's check the disk for seed data
Expand Down Expand Up @@ -302,13 +290,8 @@ export default class FileSystemCache implements CacheHandler {
return data ?? null
}

public async set(
key: string,
data: CacheHandlerValue['value'],
ctx: {
tags?: string[]
}
) {
public async set(...args: Parameters<CacheHandler['set']>) {
const [key, data, ctx] = args
memoryCache?.set(key, {
value: data,
lastModified: Date.now(),
Expand Down

0 comments on commit 4cbfe7d

Please sign in to comment.