ObservableQuery - leaks on error and unsubscribe

[konrad-garus]

I've spent a while experimenting with observable queries, and I suspect they do not quite handle errors correctly. FWIW, I was using angular2-apollo but it seems the root cause is in apollo-client itself.

Let's start with the simplest example:

const query = gql`
  query myQuery {
    authors {
      id
    }
  }
`;

apollo.watchQuery({query}).subscribe(
  resp => consume(resp.data),
  err => console.log('ERROR', err));

So far so good, as long as everything is perfectly reliable, including the server, network, etc. Not gonna happen, so I'm trying to add error handling.

As soon as anything goes wrong, the observable query emits an error and thus kills the subscription (standard rxjs behavior). Let's add retries then:

Observable
  .defer(() => apollo.watchQuery({query}))
  .retry()
  .subscribe(
    resp => consume(resp.data),
    err => console.log('ERROR', err)
  );

Now in case of error the query is retried, and the subscription at the end never actually receives an error.

Now, let's say I have a mutation with refetchQueries: ['myQuery']. Whenever it's executed, it turns out the refetch occurs as many times as there were retry attempts (+1 for the last, not-yet-broken, "open" stream).

Similarly, with updateQueries the update is executed for every failed instance as well.

Having found that, I checked the behavior after unsubscribe:

const sub1 = apollo.watchQuery({query}).subscribe();
sub1.unsubscribe();

apollo.watchQuery({query}).subscribe(
  resp => consume(resp.data),
  err => console.log('ERROR', err));

As I expected, refetchQueries as well as updateQueries made Apollo fetch/update twice, even though one of these queries had no watchers. If Apollo counted the number of active subscriptions and only executed those calls for watched streams, the behavior on error would automatically be solved as well - since in rxjs observers unsubscribe on error.

After some investigation in source code, I found QueryManager.removeObservableQuery. From a quick look it seems to be what we need here, but it doesn't appear to ever be called.

It seems to be closely related to #903 and #902, but it may not be an exact duplicate even if there is a large common part.

[helfer]

@konrad-garus thanks for the detailed report! It does indeed look like it's closely related to #902 and #903 I'm going to see if I can clean up those undead observable queries today.

[konrad-garus]

@helfer Thanks, hopefully you will be able to do this. Looking at the investigation performed in #902 and #903, it seems like it may be a deeper design issue.

As it is now, I'm afraid the observable queries are simply unusable. Old queries remain in memory and continue to haunt you. Resubscribing to an ObservableQuery that once failed returns that same error, so simply doing apollo.watchQuery(...).retry() with rxjs leads to an infinite loop (and blows the stack).

[helfer]

This should be fixed in 0.5.22.