Skip to content
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

Canceling a request #90

Open
mzgajner opened this issue Jan 4, 2021 · 7 comments · May be fixed by #106
Open

Canceling a request #90

mzgajner opened this issue Jan 4, 2021 · 7 comments · May be fixed by #106
Labels
✨ enhancement New feature or request 🛳 In PR This issue is being addressed in a PR

Comments

@mzgajner
Copy link

mzgajner commented Jan 4, 2021

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?

@logaretm
Copy link
Owner

logaretm commented Jan 4, 2021

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 logaretm added the ✨ enhancement New feature or request label Jan 4, 2021
@logaretm logaretm added the 🛳 In PR This issue is being addressed in a PR label Mar 6, 2021
@logaretm logaretm linked a pull request Mar 6, 2021 that will close this issue
@logaretm
Copy link
Owner

logaretm commented Mar 7, 2021

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.

@freddieerg
Copy link

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

@logaretm
Copy link
Owner

logaretm commented Feb 21, 2022

@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 :)

@argoroots
Copy link

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

@logaretm
Copy link
Owner

logaretm commented Jun 4, 2023

@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.

@Wouter125
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ enhancement New feature or request 🛳 In PR This issue is being addressed in a PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants