cache errors

graphql · Nov 5, 2024 · 677fbf8 · 677fbf8
1 parent 8cfab2c
commit 677fbf8
Show file tree

Hide file tree

Showing 5 changed files with 120 additions and 24 deletions.
diff --git a/src/jsutils/__tests__/withCache-test.ts b/src/jsutils/__tests__/withCache-test.ts
@@ -1,18 +1,19 @@
 import { expect } from 'chai';
 import { describe, it } from 'mocha';
 
+import { expectPromise } from '../../__testUtils__/expectPromise.js';
 import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick.js';
 
 import { isPromise } from '../isPromise.js';
 import { withCache } from '../withCache.js';
 
 describe('withCache', () => {
   it('returns asynchronously using asynchronous cache', async () => {
-    let cached: string | undefined;
+    let cached: string | Error | undefined;
     let getAttempts = 0;
     let cacheHits = 0;
     const customCache = {
-      set: async (result: string) => {
+      set: async (result: string | Error) => {
         await resolveOnNextTick();
         cached = result;
       },
@@ -46,11 +47,11 @@ describe('withCache', () => {
   });
 
   it('returns synchronously using cache with sync getter and async setter', async () => {
-    let cached: string | undefined;
+    let cached: string | Error | undefined;
     let getAttempts = 0;
     let cacheHits = 0;
     const customCache = {
-      set: async (result: string) => {
+      set: async (result: string | Error) => {
         await resolveOnNextTick();
         cached = result;
       },
@@ -80,11 +81,11 @@ describe('withCache', () => {
   });
 
   it('returns asynchronously using cache with async getter and sync setter', async () => {
-    let cached: string | undefined;
+    let cached: string | Error | undefined;
     let getAttempts = 0;
     let cacheHits = 0;
     const customCache = {
-      set: (result: string) => {
+      set: (result: string | Error) => {
         cached = result;
       },
       get: () => {
@@ -151,4 +152,74 @@ describe('withCache', () => {
     expect(getAttempts).to.equal(2);
     expect(cacheHits).to.equal(0);
   });
+
+  it('caches fn errors with sync cache', () => {
+    let cached: string | Error | undefined;
+    let getAttempts = 0;
+    let cacheHits = 0;
+    const customCache = {
+      set: (result: string | Error) => {
+        cached = result;
+      },
+      get: () => {
+        getAttempts++;
+        if (cached !== undefined) {
+          cacheHits++;
+        }
+        return cached;
+      },
+    };
+
+    const fnWithCache = withCache((): string => {
+      throw new Error('Oops');
+    }, customCache);
+
+    expect(() => fnWithCache()).to.throw('Oops');
+
+    expect(getAttempts).to.equal(1);
+    expect(cacheHits).to.equal(0);
+
+    expect(() => fnWithCache()).to.throw('Oops');
+
+    expect(getAttempts).to.equal(2);
+    expect(cacheHits).to.equal(1);
+  });
+
+  it('caches fn errors with async cache', async () => {
+    let cached: string | Error | undefined;
+    let getAttempts = 0;
+    let cacheHits = 0;
+    const customCache = {
+      set: async (result: string | Error) => {
+        await resolveOnNextTick();
+        cached = result;
+      },
+      get: () => {
+        getAttempts++;
+        if (cached !== undefined) {
+          cacheHits++;
+        }
+        return Promise.resolve(cached);
+      },
+    };
+
+    const fnWithCache = withCache((): string => {
+      throw new Error('Oops');
+    }, customCache);
+
+    const firstResultPromise = fnWithCache();
+    expect(isPromise(firstResultPromise)).to.equal(true);
+
+    await expectPromise(firstResultPromise).toRejectWith('Oops');
+    expect(getAttempts).to.equal(1);
+    expect(cacheHits).to.equal(0);
+
+    const secondResultPromise = fnWithCache();
+
+    expect(isPromise(secondResultPromise)).to.equal(true);
+
+    await expectPromise(secondResultPromise).toRejectWith('Oops');
+    expect(getAttempts).to.equal(2);
+    expect(cacheHits).to.equal(1);
+  });
 });
diff --git a/src/jsutils/withCache.ts b/src/jsutils/withCache.ts
@@ -2,14 +2,19 @@ import { isPromise } from './isPromise.js';
 import type { PromiseOrValue } from './PromiseOrValue.js';
 
 export interface FnCache<
-  T extends (...args: Array<any>) => Exclude<any, undefined>,
+  T extends (...args: Array<any>) => Exclude<any, Error | undefined>,
 > {
-  set: (result: ReturnType<T>, ...args: Parameters<T>) => PromiseOrValue<void>;
-  get: (...args: Parameters<T>) => PromiseOrValue<ReturnType<T> | undefined>;
+  set: (
+    result: ReturnType<T> | Error,
+    ...args: Parameters<T>
+  ) => PromiseOrValue<void>;
+  get: (
+    ...args: Parameters<T>
+  ) => PromiseOrValue<ReturnType<T> | Error | undefined>;
 }
 
 export function withCache<
-  T extends (...args: Array<any>) => Exclude<any, undefined>,
+  T extends (...args: Array<any>) => Exclude<any, Error | undefined>,
 >(
   fn: T,
   cache: FnCache<T>,
@@ -27,23 +32,43 @@ export function withCache<
 }
 
 function handleCacheResult<
-  T extends (...args: Array<any>) => Exclude<any, undefined>,
+  T extends (...args: Array<any>) => Exclude<any, Error | undefined>,
 >(
-  cachedResult: Awaited<ReturnType<T>> | undefined,
+  cachedResult: Awaited<ReturnType<T>> | Error | undefined,
   fn: T,
   cache: FnCache<T>,
   args: Parameters<T>,
 ): Awaited<ReturnType<T>> {
   if (cachedResult !== undefined) {
+    if (cachedResult instanceof Error) {
+      throw cachedResult;
+    }
     return cachedResult;
   }
 
-  const result = fn(...args);
+  let result;
+  try {
+    result = fn(...args);
+  } catch (error) {
+    updateResult(error, cache, args);
+    throw error;
+  }
+
+  updateResult(result, cache, args);
+  return result;
+}
+
+function updateResult<
+  T extends (...args: Array<any>) => Exclude<any, Error | undefined>,
+>(
+  result: Awaited<ReturnType<T>> | Error,
+  cache: FnCache<T>,
+  args: Parameters<T>,
+): void {
   const setResult = cache.set(result, ...args);
   if (isPromise(setResult)) {
     setResult.catch(() => {
       /* c8 ignore next */
     });
   }
-  return result;
 }
diff --git a/src/language/parser.ts b/src/language/parser.ts
@@ -120,14 +120,14 @@ export interface ParseOptions {
 
 export interface ParseCache {
   set: (
-    document: DocumentNode,
+    document: DocumentNode | Error,
     source: string | Source,
     options?: ParseOptions | undefined,
   ) => PromiseOrValue<void> | void;
   get: (
     source: string | Source,
     options?: ParseOptions | undefined,
-  ) => PromiseOrValue<DocumentNode | undefined>;
+  ) => PromiseOrValue<DocumentNode | Error | undefined>;
 }
 
 /**

diff --git a/src/validation/__tests__/validation-test.ts b/src/validation/__tests__/validation-test.ts
@@ -63,7 +63,7 @@ describe('Validate: Supports full validation', () => {
       }
     `);
 
-    let cachedErrors: ReadonlyArray<GraphQLError> | undefined;
+    let cachedErrors: ReadonlyArray<GraphQLError> | Error | undefined;
     let getAttempts = 0;
     let cacheHits = 0;
     const customCache: ValidateCache = {
@@ -119,7 +119,7 @@ describe('Validate: Supports full validation', () => {
       }
     `);
 
-    let cachedErrors: ReadonlyArray<GraphQLError> | undefined;
+    let cachedErrors: ReadonlyArray<GraphQLError> | Error | undefined;
     let getAttempts = 0;
     let cacheHits = 0;
     const customCache: ValidateCache = {
@@ -172,7 +172,7 @@ describe('Validate: Supports full validation', () => {
       }
     `);
 
-    let cachedErrors: ReadonlyArray<GraphQLError> | undefined;
+    let cachedErrors: ReadonlyArray<GraphQLError> | Error | undefined;
     let getAttempts = 0;
     let cacheHits = 0;
     const customCache: ValidateCache = {
@@ -226,7 +226,7 @@ describe('Validate: Supports full validation', () => {
       }
     `);
 
-    let cachedErrors: ReadonlyArray<GraphQLError> | undefined;
+    let cachedErrors: ReadonlyArray<GraphQLError> | Error | undefined;
     let getAttempts = 0;
     let cacheHits = 0;
     const customCache: ValidateCache = {
@@ -276,7 +276,7 @@ describe('Validate: Supports full validation', () => {
       }
     `);
 
-    let cachedErrors: ReadonlyArray<GraphQLError> | undefined;
+    let cachedErrors: ReadonlyArray<GraphQLError> | Error | undefined;
     let getAttempts = 0;
     let cacheHits = 0;
     const customCache: ValidateCache = {
@@ -329,7 +329,7 @@ describe('Validate: Supports full validation', () => {
       }
     `);
 
-    let cachedErrors: ReadonlyArray<GraphQLError> | undefined;
+    let cachedErrors: ReadonlyArray<GraphQLError> | Error | undefined;
     const customCache: ValidateCache = {
       set: async (resultedErrors) => {
         await resolveOnNextTick();

diff --git a/src/validation/validate.ts b/src/validation/validate.ts
@@ -28,7 +28,7 @@ export interface ValidateOptions {
 
 export interface ValidateCache {
   set: (
-    errors: ReadonlyArray<GraphQLError>,
+    errors: ReadonlyArray<GraphQLError> | Error,
     schema: GraphQLSchema,
     documentAST: DocumentNode,
     rules?: ReadonlyArray<ValidationRule> | undefined,
@@ -39,7 +39,7 @@ export interface ValidateCache {
     documentAST: DocumentNode,
     rules?: ReadonlyArray<ValidationRule> | undefined,
     options?: ValidateOptions | undefined,
-  ) => PromiseOrValue<ReadonlyArray<GraphQLError> | undefined>;
+  ) => PromiseOrValue<ReadonlyArray<GraphQLError> | Error | undefined>;
 }
 
 /**