From 677fbf8bafe4608c0bcd4151601e45f55f196c76 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 5 Nov 2024 13:22:46 +0200 Subject: [PATCH] cache errors --- src/jsutils/__tests__/withCache-test.ts | 83 +++++++++++++++++++-- src/jsutils/withCache.ts | 41 ++++++++-- src/language/parser.ts | 4 +- src/validation/__tests__/validation-test.ts | 12 +-- src/validation/validate.ts | 4 +- 5 files changed, 120 insertions(+), 24 deletions(-) diff --git a/src/jsutils/__tests__/withCache-test.ts b/src/jsutils/__tests__/withCache-test.ts index f3f15a2e78..d155e1c422 100644 --- a/src/jsutils/__tests__/withCache-test.ts +++ b/src/jsutils/__tests__/withCache-test.ts @@ -1,6 +1,7 @@ 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'; @@ -8,11 +9,11 @@ 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 index 2045166cda..ad33de65bf 100644 --- 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) => Exclude, + T extends (...args: Array) => Exclude, > { - set: (result: ReturnType, ...args: Parameters) => PromiseOrValue; - get: (...args: Parameters) => PromiseOrValue | undefined>; + set: ( + result: ReturnType | Error, + ...args: Parameters + ) => PromiseOrValue; + get: ( + ...args: Parameters + ) => PromiseOrValue | Error | undefined>; } export function withCache< - T extends (...args: Array) => Exclude, + T extends (...args: Array) => Exclude, >( fn: T, cache: FnCache, @@ -27,23 +32,43 @@ export function withCache< } function handleCacheResult< - T extends (...args: Array) => Exclude, + T extends (...args: Array) => Exclude, >( - cachedResult: Awaited> | undefined, + cachedResult: Awaited> | Error | undefined, fn: T, cache: FnCache, args: Parameters, ): Awaited> { 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) => Exclude, +>( + result: Awaited> | Error, + cache: FnCache, + args: Parameters, +): 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 index 85f109fd78..bf191f1d44 100644 --- 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; get: ( source: string | Source, options?: ParseOptions | undefined, - ) => PromiseOrValue; + ) => PromiseOrValue; } /** diff --git a/src/validation/__tests__/validation-test.ts b/src/validation/__tests__/validation-test.ts index ed4b997c62..7905e7cb8c 100644 --- 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 | undefined; + let cachedErrors: ReadonlyArray | Error | undefined; let getAttempts = 0; let cacheHits = 0; const customCache: ValidateCache = { @@ -119,7 +119,7 @@ describe('Validate: Supports full validation', () => { } `); - let cachedErrors: ReadonlyArray | undefined; + let cachedErrors: ReadonlyArray | Error | undefined; let getAttempts = 0; let cacheHits = 0; const customCache: ValidateCache = { @@ -172,7 +172,7 @@ describe('Validate: Supports full validation', () => { } `); - let cachedErrors: ReadonlyArray | undefined; + let cachedErrors: ReadonlyArray | Error | undefined; let getAttempts = 0; let cacheHits = 0; const customCache: ValidateCache = { @@ -226,7 +226,7 @@ describe('Validate: Supports full validation', () => { } `); - let cachedErrors: ReadonlyArray | undefined; + let cachedErrors: ReadonlyArray | Error | undefined; let getAttempts = 0; let cacheHits = 0; const customCache: ValidateCache = { @@ -276,7 +276,7 @@ describe('Validate: Supports full validation', () => { } `); - let cachedErrors: ReadonlyArray | undefined; + let cachedErrors: ReadonlyArray | Error | undefined; let getAttempts = 0; let cacheHits = 0; const customCache: ValidateCache = { @@ -329,7 +329,7 @@ describe('Validate: Supports full validation', () => { } `); - let cachedErrors: ReadonlyArray | undefined; + let cachedErrors: ReadonlyArray | Error | undefined; const customCache: ValidateCache = { set: async (resultedErrors) => { await resolveOnNextTick(); diff --git a/src/validation/validate.ts b/src/validation/validate.ts index 3d27b94d31..39dbefb26a 100644 --- a/src/validation/validate.ts +++ b/src/validation/validate.ts @@ -28,7 +28,7 @@ export interface ValidateOptions { export interface ValidateCache { set: ( - errors: ReadonlyArray, + errors: ReadonlyArray | Error, schema: GraphQLSchema, documentAST: DocumentNode, rules?: ReadonlyArray | undefined, @@ -39,7 +39,7 @@ export interface ValidateCache { documentAST: DocumentNode, rules?: ReadonlyArray | undefined, options?: ValidateOptions | undefined, - ) => PromiseOrValue | undefined>; + ) => PromiseOrValue | Error | undefined>; } /**