-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature idea: Abort pending requests #362
Comments
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. |
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 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? |
How to use this? How to use stopQuery and what is queryId? Could you make some explanation or simple example? |
+1 for abort - for things like typeahead this reduces race cases. |
Does anyone have any more information about this? I'm seeing the same issue as @SirwanAfifi. |
this works for me at 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 apollographql/apollo-client#4150 had a solution(not tested yet) |
check this discussion apollographql/apollo-client#4150 (comment) , it has a solution for canceling requests. |
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. |
@januszhou Where are you seeing Edit: It does seem to work. |
My solution turned out doesn't work the second request, looks like apollographql/apollo-client#4150 has a better solution(I haven't tested it yet) |
@januszhou Thanks! I was noticing some weirdness when I did this. I had a |
I had a problem with race condition using RxJs+Apollo and I created a simple solution, hope it will be useful. |
It looks like also 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 |
How can I get the current queryId ? |
@RLech Haven't tested but it seems like you can check |
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 |
@hueter yeah I was looking for something like this. Would be awesome when we see this in the near future. |
What would it take to implement? Is anyone already taking a try at a PR? |
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 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. |
Callbacks from the pagination components were triggering changes to the props they were passed. This in turn kicked off another fetch by the HOC and so on. Fix this by ensuring props have actually changed before making queries again. Make the queries directly from the Apollo client rather than a HOC in order to have better control over when the queries are kicked off. Ideally changes to props would cancel an existing query, but this is not really supported in apollo-client at the moment. There are partial workarounds using AbortController and .watchQuery() but neither work for us. See: https://github.com/apollographql/apollo-feature-requests/issues/40#issuecomment-489487888 This also fixes a state comparison bug in AdminIncomingMessageList and fixes an Apollo complain about a missing organization id.
Faced the same issue with I did not find a way to gracefully plug a I did the following ugly hack, which works in my case because I can keep track of the sent 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:
Really, just a "cancel inflight" would be great, since the Or a simple way to plug a (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). |
Actually, the hack above can be improved: no need to store the 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 |
For the
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();
}
};
})
);
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 Thank you for sharing! |
Any news on this one. It seems there are a lot of hacky ways to do it but any oficial PR? |
@dannycochran what is the |
This is needed! |
Because more than 2 years passed since raising this issue, if anyone is interested, I developed a library similar to Apollo - |
(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. |
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 |
@hwillson Although the custom AbortController approach does cancel the requests, it also causes this issue: In the discussion there, it was recommended not to take this approach: The suggested workaround in that issue is to use The only way to do this that I've found is an ugly hack, reaching into Apollo's internals like this: client.queryManager.inFlightLinkObservables = new Map();
// Now refetch certain queries Edit: related question, should calling |
@joshjg I don't understand why you can't kill ALL requests using |
@digeomel We have a large app using Apollo's react hooks extensively - we aren't often calling |
@joshjg apologies, it wasn't clear to me that you're using React. However, I suspect that your "ugly hack" solution can result in memory leaks, as it seems that your existing subscriptions are not properly unsubscribed. |
This is great. but do i understand correctly that this only cancels a request when a new request with the same id gets made? |
Is this issue just for manually aborting requests? We are having an issue where we are using Note, it does seem to work on component unmounting though. |
@flippidippi We're having this exact same issue. I thought this issue is about implementing this and not about manual cancellation but you seem to be right. Is there a tracking issue about aborting requests when variables are changing? |
@levrik I'm not sure? Which is why I was asking if this issue covered this. In the original issue, it says
which seems like that should be about when variables change, but most of the comments under here seem like they are addressing the more manual approach. @hwillson should this be a separate issue? |
@flippidippi Sorry. Question about if an issue already exists wasn't directed to you, thus the line break 😅 |
Hi everyone. So, after gathering the information from various sources I came up with the following solution which seems to be working good for my case. I used the base structure from here but changed the way it works so as to remove the AbortController which was creating problems (the cancelationLink was not working properly after the first cancellation, here is explained why). Actually, what I do, is store the previous observer and throw an error to it which cancels that previous request when a new request comes up for the same operation. This behavior gets applied automatically to all calls. If the user wants to prevent it for some calls he will have to pass a I am not 100% sure that it does not create some kind of memory leaks or other subscription based issues, but I have been using this for some time now, without any obvious issues. If anyone has the time please verify.
|
I there a planned in-house solution for canceling requests when the |
@hwillson Any updates regarding this? Has this been implemented? |
@hwillson Any update? Was it implemented? Or rejected? |
Still waiting for updates @hwillson, abortcontroller, unsubscribing doesn't work (angular) |
for those using reactjs, I ended up creating a new hook for each hook of apollo while waiting for official support. import {
DocumentNode,
LazyQueryExecFunction,
LazyQueryHookOptions,
OperationVariables,
QueryResult,
TypedDocumentNode,
useLazyQuery,
} from '@apollo/client';
import { mergeDeep } from '@apollo/client/utilities';
import React from 'react';
type CancellableLazyQueryResultTuple<
TData,
TVariables extends OperationVariables = OperationVariables,
> = [
LazyQueryExecFunction<TData, TVariables>,
QueryResult<TData, TVariables> & { cancelQuery: () => void },
];
export function useCancellableLazyQuery<
TData,
TVariables extends OperationVariables = OperationVariables,
>(
query: DocumentNode | TypedDocumentNode<TData, TVariables>,
options?: LazyQueryHookOptions<TData, TVariables>,
): CancellableLazyQueryResultTuple<TData, TVariables> {
const abortController = React.useRef(new AbortController());
const [execFunc, queryResult] = useLazyQuery(query, options);
const enhancedQueryExecFunc: LazyQueryExecFunction<TData, TVariables> = (options) => {
const mergedOptions = mergeDeep(options, {
context: {
fetchOptions: {
signal: abortController.current.signal,
},
},
});
return execFunc(mergedOptions);
};
const cancelQuery = () => {
abortController.current.abort();
abortController.current = new AbortController();
};
return [enhancedQueryExecFunc, { ...queryResult, cancelQuery }];
} |
This is such an important feature and still missing in the lib. Any updates on this |
Hi, Any update on this feature? |
Hey all 👋 Thanks for your patience on this issue! We've had the ability to abort requests for some time via Since this issue has been around for quite a while at this point, it feels like its become a mix of several ideas and bug reports and it has become hard to understand exactly what is being asked for. For those that have experienced bugs with this in the past, would you be willing to try and more recent version to ensure the issues have been resolved? If not, please open a bug report in the core apollo-client repo and we'd be happy to dig in further. As for the others, I see some comments asking for updates and others suggesting this feature is missing. Are you specifically looking for aborting requests in general, or are you waiting for a |
Thank you so much for finally looking into this. I find myself do need a custom function like My use case lies specifically on React Native app navigation. When a screen is out of focus, I want to cancel some in-flight network requests. |
hey there! 👋🏻 I posted about using a In the past 4 years with Apollo client, I have run into a lot of valid business logic cases (user actions such as navigating, type-aheads, or even literally clicking a "cancel" button) that are based on discrete events in the UI where I'd invoke I'm not sure if I'm interpreting Apollo's I also found a similar discussion in SWR's repo which proposes a thin wrapper around built-in fetch's AbortController but looks like you can access it at the hook's call site. |
@hueter thanks so much for the response! This is useful information!
You can absolutely do this local to the query you're using by passing the // assuming you need to keep the same controller around for every render
const controllerRef = useRef(new AbortController());
useQuery(QUERY, {
context: {
fetchOptions: {
signal: controllerRef.current.signal
}
}
});
// Abort the request when this button is clicked
<button onClick={() => controllerRef.current.abort()}>Abort request</button> If your component sticks around and could potentially abort the signal more than once, you'll need to reinstantiate the controller each time you const controllerRef = useRef(new AbortController());
const cancel = () => {
controllerRef.current.abort();
controllerRef.current = new AbortController();
}
useQuery(QUERY, {
context: {
fetchOptions: {
signal: controllerRef.current.signal
}
}
});
// Use the `cancel` function instead to ensure the controller is replaced after aborting.
// The latest `signal` will be passed the next time `useQuery` renders.
<button onClick={() => cancel()}>Abort request</button> (this is essentially the solution mentioned above) This works for any core API as well (such as @quocluongha I'm curious if the above would work in your situation? In your case, you should be able to just use that const cancel = useCallback(() => {
// ...
}, [])
useEffect(() => {
return cancel;
}, [cancel]); This should be a good solution for you without the support of a core API. All this said, I don't disagree that an exported
|
@jerelmiller Yes, the solution worked. Btw there some advanced use cases such as dealing with race condition (e.g typeahead search). As this PO said, it would be nice if the library also support cancellation behaviors like |
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 oncerace
lands into Apollo Links, like mentioned in https://www.apollographql.com/docs/link/composition.htmlThe text was updated successfully, but these errors were encountered: