diff --git a/.changeset/kind-news-agree.md b/.changeset/kind-news-agree.md new file mode 100644 index 0000000000..3e4b61bc48 --- /dev/null +++ b/.changeset/kind-news-agree.md @@ -0,0 +1,5 @@ +--- +'@graphql-yoga/plugin-response-cache': patch +--- + +Stop excessive/incorrect warn level log diff --git a/packages/plugins/response-cache/__tests__/response-cache.spec.ts b/packages/plugins/response-cache/__tests__/response-cache.spec.ts index ab038e66a7..249915b9dd 100644 --- a/packages/plugins/response-cache/__tests__/response-cache.spec.ts +++ b/packages/plugins/response-cache/__tests__/response-cache.spec.ts @@ -1,5 +1,5 @@ -import { createSchema, createYoga, Repeater } from 'graphql-yoga'; -import { cacheControlDirective } from '@envelop/response-cache'; +import { createSchema, createYoga, YogaServerInstance } from 'graphql-yoga'; +import { cacheControlDirective, ShouldCacheResultFunction } from '@envelop/response-cache'; import { useDeferStream } from '@graphql-yoga/plugin-defer-stream'; import { createInMemoryCache, useResponseCache } from '@graphql-yoga/plugin-response-cache'; @@ -197,6 +197,7 @@ it('cache a query operation per session', async () => { it('should miss cache if query variables change', async () => { const yoga = createYoga({ + logging: false, schema: createSchema({ typeDefs: /* GraphQL */ ` type Query { @@ -894,3 +895,260 @@ it('should allow to create the cache outside of the plugin', async () => { }); expect(onEnveloped).toHaveBeenCalledTimes(1); }); + +describe('shouldCacheResult', () => { + // eslint-disable-next-line @typescript-eslint/ban-types + let yoga: YogaServerInstance<{}, {}>; + let shouldCacheResultFn: ShouldCacheResultFunction | undefined; + const logging = { + debug: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + }; + + function fetch(query: string) { + return yoga.fetch('http://localhost:3000/graphql', { + method: 'POST', + headers: { + 'content-type': 'application/json', + }, + body: JSON.stringify({ query }), + }); + } + + function createYogaServer() { + return createYoga({ + logging, + schema: createSchema({ + typeDefs: /* GraphQL */ ` + ${cacheControlDirective} + + type Query { + hello: String + throws: String + } + `, + resolvers: { + Query: { + hello: () => 'world', + throws: () => { + throw new Error('DUMMY'); + }, + }, + }, + }), + plugins: [ + useResponseCache({ + session: () => null, + shouldCacheResult: shouldCacheResultFn, + includeExtensionMetadata: true, + }), + ], + }); + } + + const cacheSkip = { + extensions: { + responseCache: { + didCache: false, + hit: false, + }, + }, + }; + + const cacheSet = { + extensions: { + responseCache: { + didCache: true, + hit: false, + ttl: null, + }, + }, + }; + + const cacheHit = { + extensions: { + responseCache: { + hit: true, + }, + }, + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('when server returns errors', () => { + const query = 'query FetchError { throws }'; + const expectedResponsePayload = { + data: { throws: null }, + errors: expect.arrayContaining([expect.objectContaining({ message: 'Unexpected error.' })]), + }; + + describe('and we have no custom shouldCacheResult function', () => { + beforeEach(() => { + shouldCacheResultFn = undefined; + yoga = createYogaServer(); + }); + + it('does not cache the response (default behavior)', async () => { + const response = await fetch(query); + let body = await response.json(); + expect(body).toEqual({ + ...expectedResponsePayload, + ...cacheSkip, + }); + + const cachedResponse = await fetch(query); + body = await cachedResponse.json(); + expect(body).toEqual({ + ...expectedResponsePayload, + ...cacheSkip, + }); + expect(logging.debug).toHaveBeenCalledWith( + '[useResponseCache] Decided not to cache the response because it contains errors', + ); + }); + }); + + describe('and we want to cache it', () => { + beforeEach(() => { + shouldCacheResultFn = jest.fn(() => true); + yoga = createYogaServer(); + }); + + it('caches the error response', async () => { + const response = await fetch(query); + let body = await response.json(); + expect(body).toEqual({ + ...expectedResponsePayload, + ...cacheSet, + }); + + const cachedResponse = await fetch(query); + body = await cachedResponse.json(); + expect(body).toEqual({ + ...expectedResponsePayload, + ...cacheHit, + // NOTE: This actually returns the unmasked error DUMMY instead of the original "Unexpected error." + errors: expect.arrayContaining([expect.objectContaining({ message: 'DUMMY' })]), + }); + expect(logging.debug).not.toHaveBeenCalledWith( + '[useResponseCache] Decided not to cache the response because it contains errors', + ); + }); + }); + + describe('and we do not want to cache it', () => { + beforeEach(() => { + shouldCacheResultFn = jest.fn(() => false); + yoga = createYogaServer(); + }); + + it('does not cache the error response', async () => { + const response = await fetch(query); + let body = await response.json(); + expect(body).toEqual({ + ...expectedResponsePayload, + ...cacheSkip, + }); + + const cachedResponse = await fetch(query); + body = await cachedResponse.json(); + expect(body).toEqual({ + ...expectedResponsePayload, + ...cacheSkip, + }); + expect(logging.debug).not.toHaveBeenCalledWith( + '[useResponseCache] Decided not to cache the response because it contains errors', + ); + }); + }); + }); + + describe('when server returns no error', () => { + const query = 'query FetchHello { hello }'; + const expectedResponsePayload = { + data: { hello: 'world' }, + }; + + describe('and we have no custom shouldCacheResult function', () => { + beforeEach(() => { + shouldCacheResultFn = undefined; + yoga = createYogaServer(); + }); + + it('caches the response (default behavior)', async () => { + const response = await fetch(query); + let body = await response.json(); + expect(body).toEqual({ + ...expectedResponsePayload, + ...cacheSet, + }); + + const cachedResponse = await fetch(query); + body = await cachedResponse.json(); + expect(body).toEqual({ + ...expectedResponsePayload, + ...cacheHit, + }); + expect(logging.debug).not.toHaveBeenCalledWith( + '[useResponseCache] Decided not to cache the response because it contains errors', + ); + }); + }); + + describe('and we want to cache it', () => { + beforeEach(() => { + shouldCacheResultFn = jest.fn(() => true); + yoga = createYogaServer(); + }); + + it('caches the response', async () => { + const response = await fetch(query); + let body = await response.json(); + expect(body).toEqual({ + ...expectedResponsePayload, + ...cacheSet, + }); + + const cachedResponse = await fetch(query); + body = await cachedResponse.json(); + expect(body).toEqual({ + ...expectedResponsePayload, + ...cacheHit, + }); + expect(logging.debug).not.toHaveBeenCalledWith( + '[useResponseCache] Decided not to cache the response because it contains errors', + ); + }); + }); + + describe('and we do not want to cache it', () => { + beforeEach(() => { + shouldCacheResultFn = jest.fn(() => false); + yoga = createYogaServer(); + }); + + it('does not cache the response', async () => { + const response = await fetch(query); + let body = await response.json(); + expect(body).toEqual({ + ...expectedResponsePayload, + ...cacheSkip, + }); + + const cachedResponse = await fetch(query); + body = await cachedResponse.json(); + expect(body).toEqual({ + ...expectedResponsePayload, + ...cacheSkip, + }); + expect(logging.debug).not.toHaveBeenCalledWith( + '[useResponseCache] Decided not to cache the response because it contains errors', + ); + }); + }); + }); +}); diff --git a/packages/plugins/response-cache/package.json b/packages/plugins/response-cache/package.json index 380be2ed1f..fc9516d386 100644 --- a/packages/plugins/response-cache/package.json +++ b/packages/plugins/response-cache/package.json @@ -47,7 +47,7 @@ "graphql-yoga": "^5.0.2" }, "dependencies": { - "@envelop/response-cache": "^6.1.0" + "@envelop/response-cache": "^6.1.2" }, "devDependencies": { "graphql": "^16.6.0", diff --git a/packages/plugins/response-cache/src/index.ts b/packages/plugins/response-cache/src/index.ts index 5a68095651..eeabd75539 100644 --- a/packages/plugins/response-cache/src/index.ts +++ b/packages/plugins/response-cache/src/index.ts @@ -102,19 +102,27 @@ export function useResponseCache(options: UseResponseCacheParameter): Plugin { session: sessionFactoryForEnvelop, buildResponseCacheKey: cacheKeyFactoryForEnvelop, shouldCacheResult({ cacheKey, result }) { - const shouldCached = options.shouldCacheResult - ? options.shouldCacheResult({ cacheKey, result }) - : !result.errors?.length; - if (shouldCached) { + let shouldCache: boolean; + if (options.shouldCacheResult) { + shouldCache = options.shouldCacheResult({ cacheKey, result }); + } else { + shouldCache = !result.errors?.length; + if (!shouldCache) { + logger.debug( + '[useResponseCache] Decided not to cache the response because it contains errors', + ); + } + } + + if (shouldCache) { const extensions = (result.extensions ||= {}) as ResponseCachePluginExtensions; const httpExtensions = (extensions.http ||= {}); const headers = (httpExtensions.headers ||= {}); headers['ETag'] = cacheKey; headers['Last-Modified'] = new Date().toUTCString(); - } else { - logger.warn('[useResponseCache] Failed to cache due to errors'); } - return shouldCached; + + return shouldCache; }, }), ); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5ab9612124..c7e8f27d2c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1943,8 +1943,8 @@ importers: packages/plugins/response-cache: dependencies: '@envelop/response-cache': - specifier: ^6.1.0 - version: 6.1.0(@envelop/core@5.0.0)(graphql@16.6.0) + specifier: ^6.1.2 + version: 6.1.2(@envelop/core@5.0.0)(graphql@16.6.0) graphql-yoga: specifier: ^5.0.2 version: link:../../graphql-yoga/dist @@ -6715,8 +6715,8 @@ packages: tslib: 2.6.2 dev: false - /@envelop/response-cache@6.1.0(@envelop/core@5.0.0)(graphql@16.6.0): - resolution: {integrity: sha512-yE1H85jju/OGlbFTFfUx0dXAScewWES2UBe8UhFrMMRFTVsNwnOHFaVWQw7mwnMZjQOzahRiXPlG2Z0dxBQ6GQ==} + /@envelop/response-cache@6.1.2(@envelop/core@5.0.0)(graphql@16.6.0): + resolution: {integrity: sha512-vBX6z/TxBZSNReVn69VJVkdGHpGzHyeQNMz9LXobczl55KCZaf2inBWguSdGq+vx6PlB068GhLeg+a7kseiF1Q==} engines: {node: '>=18.0.0'} peerDependencies: '@envelop/core': ^5.0.0 @@ -8312,7 +8312,7 @@ packages: '@whatwg-node/fetch': 0.9.12 graphql: 16.6.0 isomorphic-ws: 5.0.0(ws@8.13.0) - tslib: 2.5.3 + tslib: 2.6.2 value-or-promise: 1.0.12 ws: 8.13.0 transitivePeerDependencies: @@ -20650,6 +20650,7 @@ packages: /figgy-pudding@3.5.2: resolution: {integrity: sha512-0btnI/H8f2pavGMN8w40mlSKOfTK2SVJmBfBeVIj3kNw0swwgzyRq0d5TJVOwodFmtvpPeWPN/MCcfuWF0Ezbw==} + deprecated: This module is no longer supported. dev: false /figures@1.7.0: