Skip to content
This repository has been archived by the owner on Apr 14, 2023. It is now read-only.

Subscribing to an observable causes duplicate HTTP requests #298

Open
joshjg opened this issue Dec 2, 2017 · 8 comments
Open

Subscribing to an observable causes duplicate HTTP requests #298

joshjg opened this issue Dec 2, 2017 · 8 comments

Comments

@joshjg
Copy link

joshjg commented Dec 2, 2017

Intended outcome:
I tried following the pattern of the third example here, and composing with an HttpLink:
https://www.apollographql.com/docs/link/stateless.html

Actual outcome:
All queries result in duplicate http requests being sent to the server.

How to reproduce the issue:

const httpLink = new HttpLink({ uri: foo });
const errorLink = new ApolloLink((operation, forward) => {
    const observer = forward(operation);
    observer.subscribe({ error: console.error });
    return observer;
});

const link = ApolloLink.from([
    errorLink,
    httpLink
]);
@jbaxleyiii
Copy link
Contributor

@joshjg hmm yes I think those are written incorrectly in the docs. It should be within an Observable and bound to notify it. I can update those docs, or if you feel up to it, the apollo-link-error shows the correct way to write something like that. Would you be able to update the docs matching?

@nicocrm
Copy link

nicocrm commented Feb 7, 2018

Picking up on this older issue. It has been classified as a documentation problem, but it makes the observable that is returned from the HTTP link error-prone to use in custom links. It would be great if it could be enhanced so that the pattern that is described in the documentation is possible. Ideally it would take care to only send the request once, even if subscribed to more than once.

Is there any plan to do that, or might you accept a pull request that would add that feature to the HTTP link? Or am I missing something that in fact does require the link to send the request every time the observable is subscribed to?

@evans
Copy link
Contributor

evans commented Feb 15, 2018

@nicocrm Thank you for picking this up! Take a look at #364. At least for batching, we will support sending the request only once per batch regardless of the number of subscribers(see here).

I totally agree that we should only send the request once for subscriptions that arrive in time(before and during the fetch call). We'll probably have to make some changes to how we deal with the result of the fetch call.

The root cause is due to fact that the Observable's constructor callback is invoked on every subscribe(non-obvious/intuitive in my opinion). To solve the issue, we need to queue the observers and notify them of the fetch result, then empty the queue, in case new subscribers are added after the result arrives. We would have to worry about deleting the list when unsubscribing here. It seems like an awesome change and really needed for single requests. To answer your questions, we would love to accept a pull request. Could you make one?

@naddeoa
Copy link

naddeoa commented Jun 7, 2019

I'm hitting this now on a React Native app. I'm seeing duplicate network requests being fired through the Android profiler. Looking over the conversation here I'm actually not sure if I can change something on my side to fix this or not. What is the solution here? We've got multiple links (context, custom metrics, http) and I'm not sure if this is a side effect of composing with from or something else.

@KeithGillette
Copy link

I believe I'm encountering this issue in an apollo-angular application. In attempting to track in-progress mutations, I created an ApolloLink composed with ApolloLink.from which subscribes to forward.operation in the same manner as originally described by @joshjg following the example in the Stateless Links documentation.

How may I simply be notified of the fetch result instead of duplicating the original request?

@nsnyder
Copy link

nsnyder commented Feb 19, 2020

Hey y'all,
Just opened a PR to add a warning to the documentation above because this bit me on a project. Looks like all the CI checks are running, but let me know if there's anything I need to change. It would be great to at least have a warning before I start hitting servers multiple times for every request.

Best,
Nathan

@nsnyder
Copy link

nsnyder commented Feb 20, 2020

@KeithGillette The best way at present seems to be this, from the Stateless Links documentation:

return forward(operation).map((data) => {
    // "data" will be the result. Don't use subscribe at all.
    console.log(`ending request for ${operation.operationName}`);
    return data;
  })

@KeithGillette
Copy link

Thanks, @nsnyder. That's exactly the approach we eventually got to work in order to track in-progress mutations for our application.

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

7 participants