From 27428386257859c34bf088ce7f4bf43f3221d6da Mon Sep 17 00:00:00 2001 From: Nicolas Dorseuil Date: Wed, 2 Jul 2025 15:47:39 +0200 Subject: [PATCH 1/3] change cache life for errors --- packages/gitbook/src/lib/data/errors.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/packages/gitbook/src/lib/data/errors.ts b/packages/gitbook/src/lib/data/errors.ts index c4d983ec2a..739369c4c5 100644 --- a/packages/gitbook/src/lib/data/errors.ts +++ b/packages/gitbook/src/lib/data/errors.ts @@ -98,10 +98,21 @@ export async function wrapCacheDataFetcherError( fn: () => Promise ): Promise> { const result = await wrapDataFetcherError(fn); - if (result.error && result.error.code >= 500) { - // We don't want to cache errors for too long. - // as the API might - cacheLife('minutes'); + if (result.error) { + // We only want to cache 404 errors for "long", because that's an expected error. + if (result.error.code === 404) { + cacheLife({ + stale: 60, + revalidate: 60 * 60, // 1 hour + expire: 60 * 60 * 24, // 1 day + }); + } else { + cacheLife({ + stale: 60, // This one is only for the client + revalidate: 30, // we don't want to cache it for too long, but at least 30 seconds to avoid hammering the API + expire: 90, // we want to revalidate this error after 90 seconds for sure + }); + } } return result; } From 9b5774cb7a9bc0f1bed492e929d00b4f9ebef8c2 Mon Sep 17 00:00:00 2001 From: Nicolas Dorseuil Date: Wed, 2 Jul 2025 17:39:56 +0200 Subject: [PATCH 2/3] respect cache control set by the api when possible --- packages/gitbook/src/lib/data/errors.test.ts | 121 +++++++++++++++++++ packages/gitbook/src/lib/data/errors.ts | 46 ++++++- packages/gitbook/src/lib/data/types.ts | 4 + 3 files changed, 166 insertions(+), 5 deletions(-) create mode 100644 packages/gitbook/src/lib/data/errors.test.ts diff --git a/packages/gitbook/src/lib/data/errors.test.ts b/packages/gitbook/src/lib/data/errors.test.ts new file mode 100644 index 0000000000..8667aa37e8 --- /dev/null +++ b/packages/gitbook/src/lib/data/errors.test.ts @@ -0,0 +1,121 @@ +import { describe, expect, it } from 'bun:test'; +import { GitBookAPIError } from '@gitbook/api'; +import { extractCacheControl } from './errors'; + +describe('extractCacheControl', () => { + it('should return undefined when error has no response', () => { + const error = { message: 'Test error' } as GitBookAPIError; + const result = extractCacheControl(error); + expect(result).toBeUndefined(); + }); + + it('should return undefined when response has no cache-control header', () => { + const error = new GitBookAPIError('Test error', new Response(null, {})); + + const result = extractCacheControl(error); + expect(result).toBeUndefined(); + }); + + it('should parse max-age from cache-control header', () => { + const error = new GitBookAPIError( + 'Test error', + new Response(null, { + headers: { + 'cache-control': 'max-age=3600', + }, + }) + ); + + const result = extractCacheControl(error); + expect(result).toEqual({ + maxAge: 3600, + staleWhileRevalidate: undefined, + }); + }); + + it('should parse stale-while-revalidate from cache-control header', () => { + const error = new GitBookAPIError( + 'Test error', + new Response(null, { + headers: { + 'cache-control': 'stale-while-revalidate=86400', + }, + }) + ); + + const result = extractCacheControl(error); + expect(result).toEqual({ + maxAge: undefined, + staleWhileRevalidate: 86400, + }); + }); + + it('should parse both max-age and stale-while-revalidate', () => { + const error = new GitBookAPIError( + 'Test error', + new Response(null, { + headers: { + 'cache-control': 'max-age=3600, stale-while-revalidate=86400', + }, + }) + ); + + const result = extractCacheControl(error); + expect(result).toEqual({ + maxAge: 3600, + staleWhileRevalidate: 86400, + }); + }); + + it('should return undefined for maxAge when it is 0', () => { + const error = new GitBookAPIError( + 'Test error', + new Response(null, { + headers: { + 'cache-control': 'max-age=0, stale-while-revalidate=86400', + }, + }) + ); + + const result = extractCacheControl(error); + expect(result).toEqual({ + maxAge: undefined, + staleWhileRevalidate: 86400, + }); + }); + + it('should handle complex cache-control header with multiple directives', () => { + const error = new GitBookAPIError( + 'Test error', + new Response(null, { + headers: { + 'cache-control': + 'public, max-age=3600, must-revalidate, stale-while-revalidate=86400', + }, + }) + ); + + const result = extractCacheControl(error); + expect(result).toEqual({ + maxAge: 3600, + staleWhileRevalidate: 86400, + }); + }); + + it('should return undefined when cache-control has no parseable values', () => { + const error = new GitBookAPIError( + 'Test error', + new Response(null, { + headers: { + 'cache-control': 'no-cache, no-store', + }, + }) + ); + + const result = extractCacheControl(error); + expect(result).toEqual({ + maxAge: undefined, + staleWhileRevalidate: undefined, + }); + }); +}); diff --git a/packages/gitbook/src/lib/data/errors.ts b/packages/gitbook/src/lib/data/errors.ts index 739369c4c5..2815142cfb 100644 --- a/packages/gitbook/src/lib/data/errors.ts +++ b/packages/gitbook/src/lib/data/errors.ts @@ -99,18 +99,19 @@ export async function wrapCacheDataFetcherError( ): Promise> { const result = await wrapDataFetcherError(fn); if (result.error) { - // We only want to cache 404 errors for "long", because that's an expected error. + const cacheValue = result.error.cache; + // We only want to cache 404 errors for "long", because that's an "expected" error. if (result.error.code === 404) { cacheLife({ stale: 60, - revalidate: 60 * 60, // 1 hour - expire: 60 * 60 * 24, // 1 day + revalidate: cacheValue?.maxAge ?? 60 * 60, // 1 hour + expire: cacheValue?.staleWhileRevalidate ?? 60 * 60 * 24, // 1 day }); } else { cacheLife({ stale: 60, // This one is only for the client - revalidate: 30, // we don't want to cache it for too long, but at least 30 seconds to avoid hammering the API - expire: 90, // we want to revalidate this error after 90 seconds for sure + revalidate: cacheValue?.maxAge ?? 30, // we don't want to cache it for too long, but at least 30 seconds to avoid hammering the API + expire: cacheValue?.staleWhileRevalidate ?? 90, // we want to revalidate this error after 90 seconds for sure }); } } @@ -145,6 +146,39 @@ export function ignoreDataFetcherErrors( return response; } +/** + * Extract cache control information from a GitBookAPIError. + * If the error does not have a response or no cache-control, it returns undefined. + */ +export function extractCacheControl(error: GitBookAPIError) { + try { + if (!error.response) { + return undefined; + } + + const cacheControl = error.response.headers.get('cache-control'); + if (!cacheControl) { + return undefined; + } + + const maxAgeMatch = cacheControl.match(/max-age=(\d+)/i); + const staleWhileRevalidateMatch = cacheControl.match(/stale-while-revalidate=(\d+)/i); + + const maxAge = maxAgeMatch ? Number.parseInt(maxAgeMatch[1], 10) : undefined; + const staleWhileRevalidate = staleWhileRevalidateMatch + ? Number.parseInt(staleWhileRevalidateMatch[1], 10) + : undefined; + + return { + // If maxAge is 0, we want to apply the default, not 0 + maxAge: maxAge === 0 ? undefined : maxAge, + staleWhileRevalidate, + }; + } catch { + return undefined; + } +} + /** * Get a data fetcher exposable error from a JS error. */ @@ -153,10 +187,12 @@ export function getExposableError(error: Error): DataFetcherErrorData { if (error.code >= 500) { throw error; } + const cache = extractCacheControl(error); return { code: error.code, message: error.errorMessage, + cache, }; } diff --git a/packages/gitbook/src/lib/data/types.ts b/packages/gitbook/src/lib/data/types.ts index bbcbed1573..7cfa616f13 100644 --- a/packages/gitbook/src/lib/data/types.ts +++ b/packages/gitbook/src/lib/data/types.ts @@ -3,6 +3,10 @@ import type * as api from '@gitbook/api'; export type DataFetcherErrorData = { code: number; message: string; + cache?: { + maxAge?: number; + staleWhileRevalidate?: number; + }; }; export type DataFetcherResponse = From f89f347cd53cead61709356e6d0f89f05174b1bc Mon Sep 17 00:00:00 2001 From: Nicolas Dorseuil Date: Thu, 3 Jul 2025 11:37:37 +0200 Subject: [PATCH 3/3] use parse cache control --- packages/gitbook/src/lib/data/errors.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/gitbook/src/lib/data/errors.ts b/packages/gitbook/src/lib/data/errors.ts index 2815142cfb..d73de41a95 100644 --- a/packages/gitbook/src/lib/data/errors.ts +++ b/packages/gitbook/src/lib/data/errors.ts @@ -2,6 +2,8 @@ import { GitBookAPIError } from '@gitbook/api'; import { unstable_cacheLife as cacheLife } from 'next/cache'; import type { DataFetcherErrorData, DataFetcherResponse } from './types'; +import parseCacheControl from 'parse-cache-control'; + export class DataFetcherError extends Error { constructor( message: string, @@ -160,11 +162,12 @@ export function extractCacheControl(error: GitBookAPIError) { if (!cacheControl) { return undefined; } + const parsed = parseCacheControl(cacheControl); - const maxAgeMatch = cacheControl.match(/max-age=(\d+)/i); + //parseCacheControl does not support stale-while-revalidate, so we need to parse it manually const staleWhileRevalidateMatch = cacheControl.match(/stale-while-revalidate=(\d+)/i); - const maxAge = maxAgeMatch ? Number.parseInt(maxAgeMatch[1], 10) : undefined; + const maxAge = parsed?.['max-age']; const staleWhileRevalidate = staleWhileRevalidateMatch ? Number.parseInt(staleWhileRevalidateMatch[1], 10) : undefined;