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

HttpLink's observable can not be subscribed in another link #7673

Closed
OsQu opened this issue Feb 9, 2021 · 3 comments · Fixed by #11450
Closed

HttpLink's observable can not be subscribed in another link #7673

OsQu opened this issue Feb 9, 2021 · 3 comments · Fixed by #11450

Comments

@OsQu
Copy link

OsQu commented Feb 9, 2021

Subscribing to HttpLink's observable in another link causes HttpLink to fire multiple network requests and cancelling any in-flight requests once any of the requests returns.

Intended outcome:

It should be possible to chain with HttpLink and subscribe to it's observable:

const reportErrors = (errorCallback) =>
  new ApolloLink((operation, forward) => {
    const observable = forward(operation) // This is the observable returned from HttpLink;
    observable.subscribe({ error: errorCallback });

    return observable;
  });


const httpLink = new HttpLink({..})
const client = new ApolloClient({
  cache: new InMemoryCache(),
  link: ApolloLink.from([reportErrors(console.error), httpLink])
});

The example link is copied from documentation.

Actual outcome:

Subscribing to the observable causes HttpLink to fire multiple requests and canceling any in-flight requests once any of the request returns:

image

If the subscription is removed, HttpLink does one request as expected.

How to reproduce the issue:

Here is codesandbox link reproduce the issue: https://codesandbox.io/s/bitter-feather-lztrn.

Problem

The root cause of the problem is that zen-observable's Observables are "cold". When ever observable is subscribed to, it re-runs its observer block. This causes as many requests to be made as there are subscribers.

The cancelling behaviour happens because clean up function cancels the request. The idea being that it does nothing if the request has finished but cancels the request if the observable is unsubscribed while the request is still in progress. However, since the AbortController's signal is shared between all the requests, unsubscribing any subscriber cancels rest of the requests.

Versions

This happens locally with following system:

  System:
    OS: macOS 11.1
  Binaries:
    Node: 12.14.1 - ~/.nodenv/versions/12.14.1/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 6.13.4 - ~/.nodenv/versions/12.14.1/bin/npm
  Browsers:
    Chrome: 88.0.4324.150
    Firefox: 81.0.2
    Safari: 14.0.2

If you think that the issue is valid, would you be open to receive a pull request fixing the problem?

I was thinking making the request outside the observer and returning only the contents on observer. This has the downside of making the observable "hot", but looking at the code it seems that QueryManager subscribes to the HttpLink's observable immediately, causing the request to be fired in any case. Of course the change of behaviour might break any standalone uses of HttpLink.

Another way to fix the problem would be to cache the response, keeping the observable cold while preventing multiple requests. I have a feeling that tracking the state of the request like that is more brittle and has more moving parts.

Let me know what you think!

@phryneas
Copy link
Member

phryneas commented Dec 22, 2023

Hi Oskari,
I'm sorry that is has been so long.

You are right, generally any link (not only a HttpLink) should not be subscribed to more than once, since that can cause "forking". That's not really something we can "fix", since that would also apply not only to our links, but also to any custom link any of our users has ever written, and every change we made here would be breaking in a lot of unintended ways.

I have opened #11450 to adjust our documentation example, which is - as you rightfully pointed out - wrong.

You can still intercept Link data without creating a new subscription - the example would now look like this:

const reportErrors = (errorCallback) => new ApolloLink((operation, forward) => {
    return new Observable((observer) => {
    const observable = forward(operation);
    const subscription = observable.subscribe({
      next(value) {
        observer.next(value);
      },
      error(networkError) {
        errorCallback({ networkError, operation });
        observer.error(networkError);
      },
      complete() {
        observer.complete();
      },
    });

    return () => subscription.unsubscribe();
  });
});

@jerelmiller jerelmiller linked a pull request Jan 2, 2024 that will close this issue
3 tasks
@OsQu
Copy link
Author

OsQu commented Jan 2, 2024

Thank you @phryneas. And don't worry, no time like present :-)

I don't recall how I ended up solving the issue, but now after the documentation change it should be clear.

Copy link
Contributor

github-actions bot commented Feb 2, 2024

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
For general questions, we recommend using StackOverflow or our discord server.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants