apollo-client icon indicating copy to clipboard operation
apollo-client copied to clipboard

Feature idea: Abort pending requests

Open klis87 opened this issue 5 years ago • 50 comments

Migrated from: apollographql/apollo-client#1541

I think that request aborts and other things from migated topics like options borrowed from Redux-Saga, for example takeLatest (cancelling previous request if there is new pending) could be easier implemented once race lands into Apollo Links, like mentioned in https://www.apollographql.com/docs/link/composition.html

klis87 avatar Aug 08 '18 12:08 klis87

This is really critical on systems where the pool of sockets is limited, and where CPU is limited. If the user has navigated away before we have the data, ideally the socket (or stream in the case of H/2, or subscription in the case of pubsub like redis) gets closed and cleaned up immediately so a queued request that was waiting for a free socket can get its turn. Additionally, JSON/responses for a cancelled request shouldn’t take up CPU time being parsed/deserialized. This was a key optimization with redux + RxJS allowed for in one of the teams I worked on.

matthargett avatar Dec 01 '18 23:12 matthargett

Any progress on this? In our project we have a react component to upload multiple images, now we want to add image cancelation ability, I had a look at the source code, it seems that we can pass a fetchOptions using mutation function:

const { mutation: uploadImg } = uploadImageObject;
uploadImg({
   variables: {
     file
   },
   context: {
     fetchOptions: {
      signal,
     }
   }
});

But when I trigger the abort controller, nothing happens, it actually doesn't stop pending request. Any idea?

SirwanAfifi avatar Dec 18 '18 16:12 SirwanAfifi

How to use this? How to use stopQuery and what is queryId? Could you make some explanation or simple example?

rico345100 avatar Jan 04 '19 10:01 rico345100

+1 for abort - for things like typeahead this reduces race cases.

amcdnl avatar Jan 28 '19 15:01 amcdnl

Does anyone have any more information about this? I'm seeing the same issue as @SirwanAfifi.

firasdib avatar Mar 26 '19 08:03 firasdib

this works for me at [email protected]

const abortController = new AbortController();
client.query({ 
  query,
  variables,
  context: { 
    fetchOptions: { 
      signal: abortController.signal 
    }
});
// later on
abortController.abort();

EDIT: This solution works for the first time, but it doesn't cancel for the second request, looks like https://github.com/apollographql/apollo-client/issues/4150 had a solution(not tested yet)

januszhou avatar May 06 '19 02:05 januszhou

check this discussion https://github.com/apollographql/apollo-client/issues/4150#issuecomment-487588145 , it has a solution for canceling requests.

gabor avatar May 13 '19 18:05 gabor

My case is when I use schema stitching, here is my code:

const http = ApolloLink.from([
    new RetryLink({
      attempts: {
        max: 5,
        retryIf: (error, operation: Operation) => {
          logger.error(`retry to connect error: ${error.message}`);
          return !!error;
        },
      },
      delay: {
        initial: 500,
        jitter: true,
        max: Infinity,
      },
    }),
    new HttpLink({ uri, fetch }),
  ]);

  const schemaOriginal = makeRemoteExecutableSchema({
    schema: await introspectSchema(http),
    link: http,
  });

When the remote service is down or not found, I will retry 5 times and after that, I want to stop/abort retrying.

mrdulin avatar Aug 29 '19 04:08 mrdulin

@januszhou Where are you seeing fetchOptions as an option? https://www.apollographql.com/docs/react/api/apollo-client/#ApolloClient.query

Edit: It does seem to work.

mnpenner avatar Aug 29 '19 07:08 mnpenner

@januszhou Where are you seeing fetchOptions as an option? https://www.apollographql.com/docs/react/api/apollo-client/#ApolloClient.query

Edit: It does seem to work.

My solution turned out doesn't work the second request, looks like https://github.com/apollographql/apollo-client/issues/4150 has a better solution(I haven't tested it yet)

januszhou avatar Sep 03 '19 16:09 januszhou

@januszhou Thanks! I was noticing some weirdness when I did this. I had a .finally() which was never been called after I aborted a query, probably because it was never being re-executed.

mnpenner avatar Sep 04 '19 02:09 mnpenner

I had a problem with race condition using RxJs+Apollo and I created a simple solution, hope it will be useful. graphql-express

NikitaDev14 avatar Oct 23 '19 12:10 NikitaDev14

It looks like also client.stop() helps for aborting pending requests.

const client = new ApolloClient({
	link: new HttpLink({
		uri: 'http://graphql.url'
	}),
	queryDeduplication: false
});

// some queries run

client.stop();

If you don't want to abort everything sent through the client, you can use client.queryManager.stopQuery(queryId) (this is what client.stop() uses at the end and it is reachable, but haven't tested).

bgultekin avatar Nov 15 '19 22:11 bgultekin

How can I get the current queryId ?

devdudeio avatar Dec 09 '19 19:12 devdudeio

@rlech Haven't tested but it seems like you can check client.queryManager.queries map after sending a query. You may find a better answer in the QueryManager class (https://github.com/apollographql/apollo-client/blob/65eef3730e8b3d2b66ca0fe6d88d0b579a4d31ea/packages/apollo-client/src/core/QueryManager.ts).

bgultekin avatar Dec 11 '19 14:12 bgultekin

This would be incredibly useful and powerful as part of the hooks API, for instance something like:

const { data, loading, fetchMore, refetch, cancel } = useQuery(ANY_QUERY);

useEffect(() => {
  return () => {
    // aka componentWillUnmount
    cancel();
  };
}, [cancel]);

Without this, as others have mentioned, typeahead components (e.g. ReactSelect) and other async components are susceptible to the Warning: Can't call setState (or forceUpdate) on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in the componentWillUnmount method. issue

hueter avatar Dec 11 '19 19:12 hueter

@hueter yeah I was looking for something like this. Would be awesome when we see this in the near future.

devdudeio avatar Dec 16 '19 15:12 devdudeio

What would it take to implement? Is anyone already taking a try at a PR?

TSMMark avatar Dec 19 '19 19:12 TSMMark

I need this.

We have a feature where admins can log in as different users, and when they do we invalidate and refetch the GraphQL graph by calling await client.resetStore(). This does clear and refetch all the completed queries, but the problem is that there are pending requests which will come back with data for the wrong user!

I am so sure that I can call some of the APIs on the client to invalidate the requests, but everything I've tried hasn't worked. Even if there is a low-level API I can use, this seems like a pretty natural issue to run into, so a high level API would be save the next person a lot of effort.

alflennik avatar Jan 29 '20 04:01 alflennik

Faced the same issue with fetchMore (implementing a typeahead for "search" field: without debounce on the keyboard inputs, some requests have race conditions which brought up the issue).

I did not find a way to gracefully plug a switchMap because the updateQuery function is called directly and not through an Observable chain - which is unfortunate.

I did the following ugly hack, which works in my case because I can keep track of the sent fetchMore queries. I add an artificial new random variable to all fetchMore queries I do and remember what was the last one. I accept the results in updateQuery only if the result comes from the last one.

this.lastFilterQuerySent = Math.floor(Math.random() * 100000000);

targetQuery.fetchMore({
      variables: {
        // ... other variables
        _internalSendingId: this.lastFilterQuerySent,
      },
      updateQuery: (prev, result) => {
        if (
          !result.fetchMoreResult ||
          result.variables._internalSendingId !== this.lastFilterQuerySent
        ) {
          // discard since it's not the last query
          return prev;
        }
        // ok take into account
        return result.fetchMoreResult;
      },
    });

It does the trick, but with several serious drawbacks:

  • it mixes imperative & reactive
  • if the last query fails, all other will be missed too
  • it adds a virtual new useless variable
  • in doing so, it breaks any caching heuristic
  • it does not cancel the in-flight queries, it just ignores non-last ones

Really, just a "cancel inflight" would be great, since the checkInFlight attribute properly pinpoints when a request for this query is already flying - but stop stops the query entirely, making it impossible to follow up with another call.

Or a simple way to plug a switchMap call based on when the fetchMore call takes place.

(Unsubscribing from the observable as suggested is not acceptable for my use case, since I don't own the subscribers to the data source in this context).

qortex avatar Feb 23 '20 16:02 qortex

Actually, the hack above can be improved: no need to store the internalSendingId in the query variables: taking advantage of closures and local variables scopes, it can be simplified to:

this.lastFilterQuerySent = Math.floor(Math.random() * 100000000);
const localQuerySentId = this.lastFilterQuerySent;

targetQuery.fetchMore({
      variables: {
        // ... other variables
      },
      updateQuery: (prev, result) => {
        if (
          !result.fetchMoreResult ||
          localQuerySentId !== this.lastFilterQuerySent
        ) {
          // discard since it's not the last query
          return prev;
        }
        // ok take into account
        return result.fetchMoreResult;
      },
    });

It still smells, but a bit less: caching strategies can apply normally because the variables are not touched (although it seems fetchMore calls are never cached as far as I can see in my testing examples).

qortex avatar Feb 24 '20 19:02 qortex

For the useQuery abort case, there's a fairly complex way of making this work with your Apollo Client if anyone is looking for short term relief. This is what I'm doing and it works well for the use case where I want to cancel previous queries from a text input.

  1. First, set up your apollo client with an observable that will keep track of in flight requests and if a new request comes in from the same component, cancel the previous one:
const requestLink = new ApolloLink(
  (operation, forward) =>
    new Observable(observer => {
      // Set x-CSRF token (not related to abort use case)
      let handle: ZenObservable.Subscription | undefined;
      Promise.resolve(operation)
        .then(oper => request(oper))
        .then(() => {
          handle = forward(operation).subscribe({
            next: observer.next.bind(observer),
            error: observer.error.bind(observer),
            complete: observer.complete.bind(observer),
          });
        })
        .catch(observer.error.bind(observer));

      const context = operation.getContext();
      const requestId = uuid();

      if (context.abortPreviousId) {
        const controller = new AbortController();

        operation.setContext({
          ...context,
          controller,
          fetchOptions: {
            signal: controller.signal,
          },
        });

        Object.values(inFlightOperations).forEach(operationData => {
          if (operationData?.abortPreviousId === context.abortPreviousId) {
            // If a controller exists, that means this operation should be aborted.
            operationData?.controller?.abort();
          }
        });

        inFlightOperations[requestId] = {
          controller: controller,
          abortPreviousId: context.abortPreviousId,
        };
      }

      return () => {
        // We don't need to keep around the requests, remove them once we are finished.
        delete inFlightOperations[requestId];

        if (handle) {
          handle.unsubscribe();
        }
      };
    })
);
  1. Then, you just need to pass in some unique identifier for your component. I have a wrapped useQuery function so I don't need to create the ID myself in every component, and rather, I just specify "abortPrevious" as true from components. But this works as well:
const Foo = () => {
  const abortPreviousId = useRef(uuid());
  const { data, loading, error } = useQuery(SomeQuery, {
    context: {
      // Make sure we only abort queries from this particular component, another
      // component might be using the same query simultaneously and doesn't want
      // to be aborted.
      abortPreviousId: abortPreviousId.current,
    },
  });
  return <p>bar</p>;
};

dannycochran avatar Mar 27 '20 18:03 dannycochran

@dannycochran Thank you for sharing!

dylanwulf avatar Mar 27 '20 22:03 dylanwulf

Any news on this one. It seems there are a lot of hacky ways to do it but any oficial PR?

guilhermeKodama avatar Apr 25 '20 14:04 guilhermeKodama

@dannycochran what is the request function?

blocka avatar May 24 '20 08:05 blocka

This is needed!

memark avatar Aug 14 '20 10:08 memark

Because more than 2 years passed since raising this issue, if anyone is interested, I developed a library similar to Apollo - redux-requests, but for any API - not only GraphQL - with native and automatic abort support. It has also other features, like caching, optimistic updates, automatic normalisation, Redux level SSR and so on. For people who care about requests aborts, I recommend to check it out here on github. Don't be scared with Redux, as in practice you need to write just actions and use built-in selectors.

klis87 avatar Aug 14 '20 13:08 klis87

(Solved)

After struggling for day i was finally able to get the solution. People who are looking for the answers to cancel the pending request, i have documented the POC code & solution walkthrough step by step in this stackoverflow post (Read along.)

Thanks to this discussion thread. It helped a lot to arrive at the solution.

Credits to @dannycochran & @bgultekin solutions in this thread.

Hope this helps someone out there.

nirus avatar Sep 14 '20 20:09 nirus

I also really need this to have the possibility to mark several filters as checked and only take continue with the latest requrest but cancel any earlier requests. While I was able to .unsubscribe() on a watchQuery to make sure my code isn't execute redundantly (see e.g. https://github.com/apollographql/apollo-client/issues/4150#issuecomment-500127694) the requests will still be handled by the browser. What I really need is a way to cancel the request on a network layer side.

julkue avatar Sep 16 '20 15:09 julkue

I would love to have a cancel method be exposed on an Apollo query. My use case is that I have a page with tabs, and depending on which tab is active, I load different data on the page. When the page loads, it automatically selects tab1 and starts querying using Apollo for data in my useEffect hook. However, if I pass logic to go straight to tab2 I received the error:

Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.

This is because of the following flow:

  1. Page loads with tab1 as default.
  2. Query in useEffect on tab1 sends the request
  3. Using react-router-dom state variable I switch the tab state to tab2
  4. The request has not completed yet so I received the error described above
  5. tab2 is loaded on the page.

This problem would be resolved (I think?) by allowing cancel where I could use it within my useEffect() like so to clean up unfinished queries on component unmount:

useEffect(() => {
    if (user && user.id) {
        myQuery({
            variables: {
                userId: user.id
            },
        });
    }

    return () => {
        cancel()
    }
}, [user, myQuery]);

uncvrd avatar Oct 20 '20 05:10 uncvrd