From 2a471646616e3af1b5c039e961f8d5717fad8f32 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Thu, 14 Dec 2023 13:08:11 +0100 Subject: [PATCH] Persisted Query Link: improve memory management (#11369) --- .../api-report-link_persisted-queries.md | 4 +- .changeset/thick-tips-cry.md | 9 + .size-limits.json | 4 +- .../__tests__/persisted-queries.test.ts | 63 +++- .../__tests__/react.test.tsx | 8 +- src/link/persisted-queries/index.ts | 283 +++++++++--------- 6 files changed, 232 insertions(+), 139 deletions(-) create mode 100644 .changeset/thick-tips-cry.md diff --git a/.api-reports/api-report-link_persisted-queries.md b/.api-reports/api-report-link_persisted-queries.md index dda0d3dd1db..353d48364e6 100644 --- a/.api-reports/api-report-link_persisted-queries.md +++ b/.api-reports/api-report-link_persisted-queries.md @@ -57,7 +57,9 @@ interface BaseOptions { // Warning: (ae-forgotten-export) The symbol "ApolloLink" needs to be exported by the entry point index.d.ts // // @public (undocumented) -export const createPersistedQueryLink: (options: PersistedQueryLink.Options) => ApolloLink; +export const createPersistedQueryLink: (options: PersistedQueryLink.Options) => ApolloLink & { + resetHashCache: () => void; +}; // @public (undocumented) interface DefaultContext extends Record { diff --git a/.changeset/thick-tips-cry.md b/.changeset/thick-tips-cry.md new file mode 100644 index 00000000000..407513ec1c7 --- /dev/null +++ b/.changeset/thick-tips-cry.md @@ -0,0 +1,9 @@ +--- +"@apollo/client": patch +--- + +Persisted Query Link: improve memory management +* use LRU `WeakCache` instead of `WeakMap` to keep a limited number of hash results +* hash cache is initiated lazily, only when needed +* expose `persistedLink.resetHashCache()` method +* reset hash cache if the upstream server reports it doesn't accept persisted queries diff --git a/.size-limits.json b/.size-limits.json index fb873241928..1ac8be3319e 100644 --- a/.size-limits.json +++ b/.size-limits.json @@ -1,4 +1,4 @@ { - "dist/apollo-client.min.cjs": 38589, - "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32365 + "dist/apollo-client.min.cjs": 38535, + "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32310 } diff --git a/src/link/persisted-queries/__tests__/persisted-queries.test.ts b/src/link/persisted-queries/__tests__/persisted-queries.test.ts index ea8b56e660a..32d75fe5136 100644 --- a/src/link/persisted-queries/__tests__/persisted-queries.test.ts +++ b/src/link/persisted-queries/__tests__/persisted-queries.test.ts @@ -66,7 +66,7 @@ const giveUpResponse = JSON.stringify({ errors: giveUpErrors }); const giveUpResponseWithCode = JSON.stringify({ errors: giveUpErrorsWithCode }); const multiResponse = JSON.stringify({ errors: multipleErrors }); -export function sha256(data: string) { +function sha256(data: string) { const hash = crypto.createHash("sha256"); hash.update(data); return hash.digest("hex"); @@ -151,6 +151,32 @@ describe("happy path", () => { }, reject); }); + it("clears the cache when calling `resetHashCache`", async () => { + fetchMock.post( + "/graphql", + () => new Promise((resolve) => resolve({ body: response })), + { repeat: 1 } + ); + + const hashRefs: WeakRef[] = []; + function hash(query: string) { + const newHash = new String(query); + hashRefs.push(new WeakRef(newHash)); + return newHash as string; + } + const persistedLink = createPersistedQuery({ sha256: hash }); + await new Promise((complete) => + execute(persistedLink.concat(createHttpLink()), { + query, + variables, + }).subscribe({ complete }) + ); + + await expect(hashRefs[0]).not.toBeGarbageCollected(); + persistedLink.resetHashCache(); + await expect(hashRefs[0]).toBeGarbageCollected(); + }); + itAsync("supports loading the hash from other method", (resolve, reject) => { fetchMock.post( "/graphql", @@ -517,6 +543,41 @@ describe("failure path", () => { }) ); + it.each([ + ["error message", giveUpResponse], + ["error code", giveUpResponseWithCode], + ] as const)( + "clears the cache when receiving NotSupported error (%s)", + async (_description, failingResponse) => { + fetchMock.post( + "/graphql", + () => new Promise((resolve) => resolve({ body: failingResponse })), + { repeat: 1 } + ); + fetchMock.post( + "/graphql", + () => new Promise((resolve) => resolve({ body: response })), + { repeat: 1 } + ); + + const hashRefs: WeakRef[] = []; + function hash(query: string) { + const newHash = new String(query); + hashRefs.push(new WeakRef(newHash)); + return newHash as string; + } + const persistedLink = createPersistedQuery({ sha256: hash }); + await new Promise((complete) => + execute(persistedLink.concat(createHttpLink()), { + query, + variables, + }).subscribe({ complete }) + ); + + await expect(hashRefs[0]).toBeGarbageCollected(); + } + ); + itAsync("works with multiple errors", (resolve, reject) => { fetchMock.post( "/graphql", diff --git a/src/link/persisted-queries/__tests__/react.test.tsx b/src/link/persisted-queries/__tests__/react.test.tsx index 07c3fe7375e..b05e7d98f32 100644 --- a/src/link/persisted-queries/__tests__/react.test.tsx +++ b/src/link/persisted-queries/__tests__/react.test.tsx @@ -4,6 +4,7 @@ import * as ReactDOM from "react-dom/server"; import gql from "graphql-tag"; import { print } from "graphql"; import fetchMock from "fetch-mock"; +import crypto from "crypto"; import { ApolloProvider } from "../../../react/context"; import { InMemoryCache as Cache } from "../../../cache/inmemory/inMemoryCache"; @@ -12,7 +13,12 @@ import { createHttpLink } from "../../http/createHttpLink"; import { graphql } from "../../../react/hoc/graphql"; import { getDataFromTree } from "../../../react/ssr/getDataFromTree"; import { createPersistedQueryLink as createPersistedQuery, VERSION } from ".."; -import { sha256 } from "./persisted-queries.test"; + +function sha256(data: string) { + const hash = crypto.createHash("sha256"); + hash.update(data); + return hash.digest("hex"); +} // Necessary configuration in order to mock multiple requests // to a single (/graphql) endpoint diff --git a/src/link/persisted-queries/index.ts b/src/link/persisted-queries/index.ts index fa4c64bac58..95c4129c802 100644 --- a/src/link/persisted-queries/index.ts +++ b/src/link/persisted-queries/index.ts @@ -12,6 +12,7 @@ import type { import { Observable, compact, isNonEmptyArray } from "../../utilities/index.js"; import type { NetworkError } from "../../errors/index.js"; import type { ServerError } from "../utils/index.js"; +import { WeakCache } from "@wry/caches"; export const VERSION = 1; @@ -93,7 +94,10 @@ function operationDefinesMutation(operation: Operation) { export const createPersistedQueryLink = ( options: PersistedQueryLink.Options ) => { - const hashesByQuery = new WeakMap>(); + let hashesByQuery: WeakCache> | undefined; + function resetHashCache() { + hashesByQuery = undefined; + } // Ensure a SHA-256 hash function is provided, if a custom hash // generation function is not provided. We don't supply a SHA-256 hash // function by default, to avoid forcing one as a dependency. Developers @@ -135,149 +139,160 @@ export const createPersistedQueryLink = ( // what to do with the bogus query. return getHashPromise(query); } + if (!hashesByQuery) { + hashesByQuery = + new WeakCache(/** TODO: decide on a maximum size (will do all max sizes in a combined separate PR) */); + } let hash = hashesByQuery.get(query)!; if (!hash) hashesByQuery.set(query, (hash = getHashPromise(query))); return hash; } - return new ApolloLink((operation, forward) => { - invariant( - forward, - "PersistedQueryLink cannot be the last link in the chain." - ); - - const { query } = operation; - - return new Observable((observer: Observer) => { - let subscription: ObservableSubscription; - let retried = false; - let originalFetchOptions: any; - let setFetchOptions = false; - const maybeRetry = ( - { - response, - networkError, - }: { response?: ExecutionResult; networkError?: ServerError }, - cb: () => void - ) => { - if (!retried && ((response && response.errors) || networkError)) { - retried = true; - - const graphQLErrors: GraphQLError[] = []; - - const responseErrors = response && response.errors; - if (isNonEmptyArray(responseErrors)) { - graphQLErrors.push(...responseErrors); - } - - // Network errors can return GraphQL errors on for example a 403 - let networkErrors; - if (typeof networkError?.result !== "string") { - networkErrors = - networkError && - networkError.result && - (networkError.result.errors as GraphQLError[]); - } - if (isNonEmptyArray(networkErrors)) { - graphQLErrors.push(...networkErrors); - } - - const disablePayload: ErrorResponse = { + return Object.assign( + new ApolloLink((operation, forward) => { + invariant( + forward, + "PersistedQueryLink cannot be the last link in the chain." + ); + + const { query } = operation; + + return new Observable((observer: Observer) => { + let subscription: ObservableSubscription; + let retried = false; + let originalFetchOptions: any; + let setFetchOptions = false; + const maybeRetry = ( + { response, networkError, - operation, - graphQLErrors: - isNonEmptyArray(graphQLErrors) ? graphQLErrors : void 0, - meta: processErrors(graphQLErrors), - }; - - // if the server doesn't support persisted queries, don't try anymore - supportsPersistedQueries = !disable(disablePayload); - - // if its not found, we can try it again, otherwise just report the error - if (retry(disablePayload)) { - // need to recall the link chain - if (subscription) subscription.unsubscribe(); - // actually send the query this time - operation.setContext({ - http: { - includeQuery: true, - includeExtensions: supportsPersistedQueries, - }, - fetchOptions: { - // Since we're including the full query, which may be - // large, we should send it in the body of a POST request. - // See issue #7456. - method: "POST", - }, - }); - if (setFetchOptions) { - operation.setContext({ fetchOptions: originalFetchOptions }); + }: { response?: ExecutionResult; networkError?: ServerError }, + cb: () => void + ) => { + if (!retried && ((response && response.errors) || networkError)) { + retried = true; + + const graphQLErrors: GraphQLError[] = []; + + const responseErrors = response && response.errors; + if (isNonEmptyArray(responseErrors)) { + graphQLErrors.push(...responseErrors); } - subscription = forward(operation).subscribe(handler); - return; - } - } - cb(); - }; - const handler = { - next: (response: ExecutionResult) => { - maybeRetry({ response }, () => observer.next!(response)); - }, - error: (networkError: ServerError) => { - maybeRetry({ networkError }, () => observer.error!(networkError)); - }, - complete: observer.complete!.bind(observer), - }; - - // don't send the query the first time - operation.setContext({ - http: { - includeQuery: !supportsPersistedQueries, - includeExtensions: supportsPersistedQueries, - }, - }); + // Network errors can return GraphQL errors on for example a 403 + let networkErrors; + if (typeof networkError?.result !== "string") { + networkErrors = + networkError && + networkError.result && + (networkError.result.errors as GraphQLError[]); + } + if (isNonEmptyArray(networkErrors)) { + graphQLErrors.push(...networkErrors); + } - // If requested, set method to GET if there are no mutations. Remember the - // original fetchOptions so we can restore them if we fall back to a - // non-hashed request. - if ( - useGETForHashedQueries && - supportsPersistedQueries && - !operationDefinesMutation(operation) - ) { - operation.setContext( - ({ fetchOptions = {} }: { fetchOptions: Record }) => { - originalFetchOptions = fetchOptions; - return { - fetchOptions: { - ...fetchOptions, - method: "GET", - }, + const disablePayload: ErrorResponse = { + response, + networkError, + operation, + graphQLErrors: + isNonEmptyArray(graphQLErrors) ? graphQLErrors : void 0, + meta: processErrors(graphQLErrors), }; + + // if the server doesn't support persisted queries, don't try anymore + supportsPersistedQueries = !disable(disablePayload); + if (!supportsPersistedQueries) { + // clear hashes from cache, we don't need them anymore + resetHashCache(); + } + + // if its not found, we can try it again, otherwise just report the error + if (retry(disablePayload)) { + // need to recall the link chain + if (subscription) subscription.unsubscribe(); + // actually send the query this time + operation.setContext({ + http: { + includeQuery: true, + includeExtensions: supportsPersistedQueries, + }, + fetchOptions: { + // Since we're including the full query, which may be + // large, we should send it in the body of a POST request. + // See issue #7456. + method: "POST", + }, + }); + if (setFetchOptions) { + operation.setContext({ fetchOptions: originalFetchOptions }); + } + subscription = forward(operation).subscribe(handler); + + return; + } } - ); - setFetchOptions = true; - } - - if (supportsPersistedQueries) { - getQueryHash(query) - .then((sha256Hash) => { - operation.extensions.persistedQuery = { - version: VERSION, - sha256Hash, - }; - subscription = forward(operation).subscribe(handler); - }) - .catch(observer.error!.bind(observer)); - } else { - subscription = forward(operation).subscribe(handler); - } - - return () => { - if (subscription) subscription.unsubscribe(); - }; - }); - }); + cb(); + }; + const handler = { + next: (response: ExecutionResult) => { + maybeRetry({ response }, () => observer.next!(response)); + }, + error: (networkError: ServerError) => { + maybeRetry({ networkError }, () => observer.error!(networkError)); + }, + complete: observer.complete!.bind(observer), + }; + + // don't send the query the first time + operation.setContext({ + http: { + includeQuery: !supportsPersistedQueries, + includeExtensions: supportsPersistedQueries, + }, + }); + + // If requested, set method to GET if there are no mutations. Remember the + // original fetchOptions so we can restore them if we fall back to a + // non-hashed request. + if ( + useGETForHashedQueries && + supportsPersistedQueries && + !operationDefinesMutation(operation) + ) { + operation.setContext( + ({ fetchOptions = {} }: { fetchOptions: Record }) => { + originalFetchOptions = fetchOptions; + return { + fetchOptions: { + ...fetchOptions, + method: "GET", + }, + }; + } + ); + setFetchOptions = true; + } + + if (supportsPersistedQueries) { + getQueryHash(query) + .then((sha256Hash) => { + operation.extensions.persistedQuery = { + version: VERSION, + sha256Hash, + }; + subscription = forward(operation).subscribe(handler); + }) + .catch(observer.error!.bind(observer)); + } else { + subscription = forward(operation).subscribe(handler); + } + + return () => { + if (subscription) subscription.unsubscribe(); + }; + }); + }), + { resetHashCache } + ); };