villus icon indicating copy to clipboard operation
villus copied to clipboard

Canceling a request

Open mzgajner opened this issue 4 years ago • 7 comments

We have a use-case that would require canceling requests - some are triggered explicitly using execute and some are triggered dynamically, by mutating responsive query variables.

Is there any way to do this with Villus? If not, how do you feel about such a feature, would you consider implementing it/accept a PR implementing it or do you think it's not a good fit for the automagic approach the library generally takes?

mzgajner avatar Jan 04 '21 17:01 mzgajner

I don't mind seeing this, maybe useQuery could return a cancel function that uses the AbortControlelr API?

I think this is tricky to implement tho, as the pipeline of plugins isn't really aware of the caller and the caller isn't aware of what happens inside, which might require some fiddling.

I will mark this as an enhancement, for now, will try to get around to it when I can. Feel free to PR it or share your ideas.

logaretm avatar Jan 04 '21 21:01 logaretm

Update: at the moment there is a PR to get this functionality out, this is the proposed behavior

  • All queries/mutations are abortable with abort function
  • If subsequent requests from the same useQuery or useMutation instances are executed while others are in the pipeline, the previous requests will be aborted automatically.
  • The abort feature works if the AbortController API is supported in the browser executing the code, otherwise it would just ignore the results of aborted queries/mutations and will not update the state with their result.

The only problems left to work out is to determine what happens if:

  • A deduped request gets aborted, what happens to the other deduped requests?
  • A batched request gets aborted since you simply aren't aborting a single operation, what happens to the other batched operations?

Looking at how apollo and other clients handle this, it doesn't seem that they offer this out of the box and user's handling is required for these cases, but they do seem to cancel the request completely which might cause issues with both scenarios, the community seems to turn off these features when using cancellable queries/mutations.

I'm thinking of introducing 2 layers of cancellation, a hard cancellation with AbortController and a soft "just ignore the query", and for these cases I would go with the soft cancellation strategy, while opting for the first if its safe to do so.

logaretm avatar Mar 07 '21 19:03 logaretm

Has there been any movement on this since above? I also have a similar need to explicitly cancel a request.

freddieerg avatar Feb 19 '22 11:02 freddieerg

@freddieerg I still haven't worked out the caveats of calling cancel with dedup or batch enabled, also having a hard time finding the time to work on this piece.

The specs for this so far looks like this:

  • Should cancel the network request with abort controller signal if it serves exactly one operation (not deduped or batched)
  • Should not cancel the network request if it contains more than 1 operation (both for dedup and batch).
  • Should ignore the result of the response if the same deduped operation was canceled.
  • Should not ignore the result of the response if a different deduped operation was canceled.

This would require writing the tests and logic to do this, and also writing the documentation to make it clear in these cases how the cancellation works.

PRs are welcome here as well :)

logaretm avatar Feb 21 '22 21:02 logaretm

@logaretm, will this also work with useSubscription? Or is there already option to cancel (not just pause) subscriptions?

argoroots avatar Jun 01 '23 10:06 argoroots

@argoroots Nope, this is only for mutations/queries and if you use HTTP for subscriptions it could work. However there is no way to avoid the issues I mentioned at the moment. I may need to revisit this at another time when I can focus on it.

For subs, I realize there is no way to completely stop it, maybe we can expose something quickly, the logic is already there. It just needs exposing on useSubscription return object.

logaretm avatar Jun 04 '23 22:06 logaretm

Can we still get close to be exposed? Would be a nice to have so we don't have open subscriptions if we do not need them.

Wouter125 avatar Nov 28 '23 09:11 Wouter125