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

fetchMore leaks watched queries #2286

Closed
bgentry opened this issue Oct 11, 2017 · 16 comments
Closed

fetchMore leaks watched queries #2286

bgentry opened this issue Oct 11, 2017 · 16 comments

Comments

@bgentry
Copy link
Contributor

bgentry commented Oct 11, 2017

I'm using fetchMore on an ObservableQuery that I'm already subscribed to. I'm not starting any new watchQueries. However, the Apollo Chrome inspector shows that each time I use fetchMore to fetch another page of results, a new "watched query" appears in the list. These remain even after I unsubscribe from my original subscription. Is this expected? Is there something else I need to do to clean up old watchQueries when using fetchMore?

This is all happening in ember-apollo-client, so it's entirely possible there's a bug or behavior in that integration specific to fetchMore because this is the first time I'm trying to use it. Digging through react-apollo though I couldn't find anything specific to deal with this sort of thing. And there doesn't appear to be any documentation describing the behavior I'm seeing.

Side note: I am the maintainer of ember-apollo-client, so I'm quite interested in understanding and fixing this issue.

Intended outcome:

Use fetchMore to load the next page of results. Expected that the "watched queries" list does not grow because I'm not actually starting a new query or making a new subscription to an ObservableQuery.

Actual outcome:

The Apollo Chrome inspector shows that each time I use fetchMore to fetch another page of results, a new "watched query" appears in the list. These remain even after I unsubscribe from my original subscription (i.e. when I change routes, all of them stick around except one).

pagination-watched-queries

Is this expected? Is there something else I need to do to clean up old watchQueries when using fetchMore?

How to reproduce the issue:

Watch the Apollo Chrome inspector's "watched queries" list while you use fetchMore in any app that does so.

Version

  • apollo-client@1.9.3
@chris08002
Copy link

Same issue here with

    "apollo-angular": "0.13.0",
    "apollo-client": "^1.7.0",

@miquelferrerllompart
Copy link

I have open issue with the same problem.
kamilkisiela/apollo-angular#376

@stale
Copy link

stale bot commented Nov 13, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions to Apollo Client!

@bgentry
Copy link
Contributor Author

bgentry commented Nov 13, 2017

I haven't yet upgraded to Apollo Client 2.0. Can anybody confirm whether this is still a problem there? AFAIK it has not been fixed.

@chris08002
Copy link

@bgentry same problem with apollo client 2.0. Any suggestion how to solve this. I tried to look into the apollo code, but don‘t really understand it. This is kind of a blocking issue for us.

@stale
Copy link

stale bot commented Dec 5, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions to Apollo Client!

@bgentry
Copy link
Contributor Author

bgentry commented Dec 5, 2017

Still an issue afaik. Is there a label that can be put on this so it's acknowledged as a real issue and the bot doesn't try to auto-close it every 3 weeks?

@stale stale bot removed the no-recent-activity label Dec 5, 2017
@chris08002
Copy link

The issue is still the same. We went through the hassle of installing 2.0 and the problem continues. We have build some workaround now but it is not a real solution.

@raysuelzer
Copy link

Is there any update on this? It's a pretty yucky leak.

@mmun
Copy link

mmun commented May 30, 2018

Can someone confirm whether this behaviour is expected or not?

@stubailo
Copy link
Contributor

Hi! Could someone send a PR with a failing test for this? That would be a great start to a fix!

@hwillson
Copy link
Member

We haven't heard back regarding a reproduction, so closing for now. Thanks!

@bgentry
Copy link
Contributor Author

bgentry commented Jul 27, 2018

@hwillson do you have reason to believe this is actually fixed? The fetchMore function was leaking 100% of the time last I checked, trivially verifiable with the Apollo Chrome extension and demonstrated by the original post.

Unless that’s been fixed the function is just not safe to use in a production app.

@hwillson
Copy link
Member

@bgentry I'm not sure if this has been fixed (or if this is an issue). I'm just triaging the issue backlog, and anything with a reproduction-needed label that hasn't received a repro in a timely manner, is to be closed. We're ramping up our repo maintenance in several areas to make sure we address issues in more timely manner, and we're isolating feature requests (eventually for all repos) so we know where we should focus next. This means we're closing out issues that have been open for a while, need a reproduction, and haven't received one.

That being said, is there any chance you can demonstrate this happening in the form of a small runnable reproduction? Attempting to re-create this by using Apollo Dev Tools and watching the inspector doesn't necessarily point to this being an Apollo Client problem. E.g., this could be an Apollo Dev Tools issue. If you're able to put together a reproduction using apollo-client and fetchMore that shows this leak, we'll definitely re-open this issue and investigate. Thanks!

@bgentry
Copy link
Contributor Author

bgentry commented Jul 28, 2018

Last time I tried to put a reproduction together I wasn’t actually sure where to look for similar tests or how to figure out the list of active queries. If there are existing tests for fetchMore and it’s easy to get a list of active watches queries, then a repo would be easy.

Maybe one of the others can chime in on this. I don’t actively use that function anymore because it was (is?) fundamentally broken for so long.

In either case I don’t know the Apollo internals at all these days so it wouldn’t be that easy for me to dig into this.

@mcbroh
Copy link

mcbroh commented Jul 31, 2018

capture

Having the same problem here, all help will be kindly appreciated.

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

No branches or pull requests

8 participants