From 345fbcb5c7322bd55b630489047085872840479b Mon Sep 17 00:00:00 2001 From: schroda <50052685+schroda@users.noreply.github.com> Date: Sat, 11 Nov 2023 23:42:58 +0100 Subject: [PATCH] Fix/apollo client spamming infinite requets on failure (#450) * Remove abort controller by default from requests Apollo client is, for whatever reason, considering it a new hook call when passing an abort controller to be passed to the actual request. On request failures this then causes infinite requests due to it being a new hook call. * Move side effect into useEffect This was missed and started causing issues after the changes of 60429d9d98544e1e49a112aacf77bdbb831740ec. Due to the changes, the component did not re-render unnecessarily which caused the component not to re-render with the fetched data * Reset fetching initial pages state The state was never reset, thus, in case of an error, the initial pages fetch was never triggered again --- src/lib/requests/RequestManager.ts | 35 +++++++++++++++++++----------- src/screens/SearchAll.tsx | 1 + src/screens/Updates.tsx | 1 - 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/lib/requests/RequestManager.ts b/src/lib/requests/RequestManager.ts index 91a9c0d0..b289ca39 100644 --- a/src/lib/requests/RequestManager.ts +++ b/src/lib/requests/RequestManager.ts @@ -225,9 +225,11 @@ type CustomApolloOptions = { * which - I assume - results in apollo to handle this as a completely new hook call. * Due to this, when e.g. calling "fetchMore", "loading" and "networkStatus" do not get updated when enabling "notifyOnNetworkStatusChange". * - * By not passing an abort signal, the states get correctly updated, BUT it won't be possible to abort the request. + * It also causes apollo to spam requests in case of request failures on every rerender. + * + * Instead of adding the abort signal by default, it has to be added manually which will cause these stated issues (and potentially more?) */ - omitAbortSignal?: boolean; + addAbortSignal?: boolean; }; type QueryOptions = Partial< ApolloQueryOptions @@ -251,7 +253,7 @@ type ApolloPaginatedMutationOptions = Partial< ApolloSubscriptionHookOptions > & - Omit & { omitAbortSignal?: never }; + Omit & { addAbortSignal?: never }; type AbortableRequest = { abortRequest: AbortController['abort'] }; @@ -552,16 +554,21 @@ export class RequestManager { areFetchingInitialPages: boolean, areInitialPagesFetched: boolean, setRevalidationDone: (isDone: boolean) => void, - cacheInitialPagesKey: string, + cacheFetchingInitialPagesKey: string, getVariablesFor: (page: number) => Variables, initialPages: number, fetchPage: (page: number) => Promise>, hasNextPage: (result: FetchResult) => boolean, ): void { - const shouldFetchInitialPages = !options?.skipRequest && !areFetchingInitialPages && !areInitialPagesFetched; - if (shouldFetchInitialPages) { + useEffect(() => { + const shouldFetchInitialPages = + !options?.skipRequest && !areFetchingInitialPages && !areInitialPagesFetched; + if (!shouldFetchInitialPages) { + return; + } + setRevalidationDone(true); - this.cache.cacheResponse(cacheInitialPagesKey, getVariablesFor(0), true); + this.cache.cacheResponse(cacheFetchingInitialPagesKey, getVariablesFor(0), true); const loadInitialPages = async (initialPage: number) => { const areAllPagesFetched = initialPage > initialPages; @@ -576,8 +583,10 @@ export class RequestManager { } }; - loadInitialPages(1); - } + loadInitialPages(1).finally(() => + this.cache.cacheResponse(cacheFetchingInitialPagesKey, getVariablesFor(0), false), + ); + }, [!options?.skipRequest, !areFetchingInitialPages, !areInitialPagesFetched]); } private returnPaginatedMutationResult( @@ -722,7 +731,7 @@ export class RequestManager { context: { ...options?.context, fetchOptions: { - signal: options?.omitAbortSignal ? undefined : signal, + signal: options?.addAbortSignal ? signal : undefined, ...options?.context?.fetchOptions, }, }, @@ -738,7 +747,7 @@ export class RequestManager { context: { ...options?.context, fetchOptions: { - signal: options?.omitAbortSignal ? undefined : signal, + signal: options?.addAbortSignal ? signal : undefined, ...options?.context?.fetchOptions, }, }, @@ -754,7 +763,7 @@ export class RequestManager { context: { ...options?.context, fetchOptions: { - signal: options?.omitAbortSignal ? undefined : signal, + signal: options?.addAbortSignal ? signal : undefined, ...options?.context?.fetchOptions, }, }, @@ -770,7 +779,7 @@ export class RequestManager { context: { ...options?.context, fetchOptions: { - signal: options?.omitAbortSignal ? undefined : signal, + signal: options?.addAbortSignal ? signal : undefined, ...options?.context?.fetchOptions, }, }, diff --git a/src/screens/SearchAll.tsx b/src/screens/SearchAll.tsx index 1fa62daf..19bd09fd 100644 --- a/src/screens/SearchAll.tsx +++ b/src/screens/SearchAll.tsx @@ -100,6 +100,7 @@ const SourceSearchPreview = React.memo( const { id, displayName, lang } = source; const [, results] = requestManager.useSourceSearch(id, searchString ?? '', undefined, 1, { skipRequest: !searchString, + addAbortSignal: true, }); const { data: searchResult, isLoading, error, abortRequest } = results[0]!; const mangas = searchResult?.fetchSourceManga.mangas ?? []; diff --git a/src/screens/Updates.tsx b/src/screens/Updates.tsx index bd6deafe..bd6c9c8a 100644 --- a/src/screens/Updates.tsx +++ b/src/screens/Updates.tsx @@ -108,7 +108,6 @@ export const Updates: React.FC = () => { } = requestManager.useGetRecentlyUpdatedChapters(undefined, { fetchPolicy: 'cache-and-network', notifyOnNetworkStatusChange: true, - omitAbortSignal: true, }); const hasNextPage = !!chapterUpdateData?.chapters.pageInfo.hasNextPage; const endCursor = chapterUpdateData?.chapters.pageInfo.endCursor;