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

Unsubscribing from a query does not abort network requests #6985

Closed
knedev42 opened this issue Sep 6, 2020 · 4 comments · Fixed by #7165
Closed

Unsubscribing from a query does not abort network requests #6985

knedev42 opened this issue Sep 6, 2020 · 4 comments · Fixed by #7165

Comments

@knedev42
Copy link

knedev42 commented Sep 6, 2020

Aborting network requests used to work in apollo-client 2.5.1, where they could be cancelled by either unmounting a component or unsubscribing from an observable query.

I skimmed through the code and it seems that if the browser supports AbortController, the http link should cancel pending requests when you unsubscribe from it, but it never does 🤷

Intended outcome:

When you subscribe to a query and later unsubscribe from it, while the network request is still in-flight, it should be aborted.

Actual outcome:

Network requests are never aborted.

Versions

npmPackages:
@apollo/client: ^3.1.3 => 3.1.3

@Torsten85
Copy link
Contributor

Torsten85 commented Sep 30, 2020

I've seen the same bug.

Here is a sandbox with @apollo/link 3.2.1: https://codesandbox.io/s/apollo-client-321-unsubscribe-h76nh
And here is the same code with apollo-link: 2.5.1: https://codesandbox.io/s/apollo-client-251-unsubscribe-bh0mc

The unsubscribe method gets never called in 3.2.1 and thus not aborting the http request.

I've digged through the code and I think the problem is the following:

Would be great if someone could have a look at this.

@dylanwulf
Copy link
Contributor

I actually submitted an issue about this over a year ago (#4955) and unfortunately it seems like it hasn't been resolved yet

@Torsten85
Copy link
Contributor

Oh, did not find your ticket.

I think this should get way more attention. It's an ugly bug with an easy fix.

javier-garcia-meteologica added a commit to javier-garcia-meteologica/apollo-client that referenced this issue Oct 14, 2020
javier-garcia-meteologica added a commit to javier-garcia-meteologica/apollo-client that referenced this issue Oct 14, 2020
benjamn pushed a commit to javier-garcia-meteologica/apollo-client that referenced this issue Oct 14, 2020
benjamn added a commit to javier-garcia-meteologica/apollo-client that referenced this issue Oct 14, 2020
When there are multiple observers subscribed to a Concast, messages should
be delivered in the order the observers were added.

The shadowObservers approach notified any cleanup observers after any
normal observers, potentially reordereding message delivery. I'm not sure
that reordering was necessarily "wrong," but it would be a very tricky
problem to track down for anyone who might be affected by it.

However, if any other non-cleanup observers have been added, and the last
observer removed is a cleanup observer, then we should unsubscribe from
this.sub, to preserve the fix for apollographql#6985.
benjamn added a commit to javier-garcia-meteologica/apollo-client that referenced this issue Oct 14, 2020
When there are multiple observers subscribed to a Concast, messages should
be delivered in the order the observers were added.

The shadowObservers approach notified any cleanup observers after any
normal observers, potentially reordereding message delivery. I'm not sure
that reordering was necessarily "wrong," but it would be a very tricky
problem to track down for anyone who might be affected by it.

However, if any other non-cleanup observers have been added, and the last
observer removed is a cleanup observer, then we should unsubscribe from
this.sub, to preserve the fix for apollographql#6985.
javier-garcia-meteologica added a commit to javier-garcia-meteologica/apollo-client that referenced this issue Oct 15, 2020
javier-garcia-meteologica added a commit to javier-garcia-meteologica/apollo-client that referenced this issue Oct 15, 2020
benjamn added a commit that referenced this issue Oct 16, 2020
…leanup2

Fix cleanup observer blocking unsubscribe, part 2. Fixes #6985.
@brendanmoore
Copy link

I believe that part of the issue will be fixed in 3.3+. If you use your own watchQuery and manually unsubscribe. I am using this approach: https://gist.github.com/brendanmoore/8cd239c5b35983936581b61922b09b2e and it works flawlessly in 3.3.0-beta.16 as Apollo does use its own AbortController internally when you unsubscribe - (when using HttpLink).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants