search

apollographql / apollo-link

:link: Interface for fetching and modifying control flow of GraphQL requests
https://www.apollographql.com/docs/link/
MIT License
1.44k stars 344 forks source link

Two request can get deduplicated if same query and vars but different headers #920

Open robermorales opened 5 years ago

robermorales commented 5 years ago

Two queries can get wrongly deduplicated if they are requesting the same query and vars to the same host, but with different HTTP headers. Perhaps we can include all or some request Headers into the getKey of the operation type, so that case is well covered and never the second request got hidden with the result of the first one.

https://github.com/apollographql/apollo-link/blob/089f71be5ea6a2ccd465751a500b16b97dbc23aa/packages/apollo-link/src/linkUtils.ts#L127

I do not know even how to implement this, but first: is this idea logic, reliable, and feasible??

Thanks,

JoviDeCroock commented 5 years ago

Could you provide a use case for this, can't really see any case for this yet. That's probably just me, like do you mean when your token has changed or?

In all sincerity, can't really estimate the impact at this point.

robermorales commented 5 years ago

I reached this situation in my current project. Yes, it is about auth headers, but could be another header.

Neither the HTTP RFC or the graphql specification says nothing against doing several requests at the same time with different headers.

Actually, the headers I see that get wrongly deduped are targeting the nginx proxy between the graphql client and the server, they have nothing to do with graphql, but with other layers of the communication.

Anyway, if we choose carefully the implementation, I think that the change could be harmless for every other use case.

JoviDeCroock commented 5 years ago

Yeah was not doubting your need for it or anything, just trying to see if it was a really specific scenario or not, no worries. Let's look at it, I think it can imply some issues though. Not 100% sure

robermorales commented 5 years ago

Cool!

Let me know if I can help with the implementation.

Thanks!!!

JoviDeCroock commented 5 years ago

In the meanwhile you can probably use context: { forceFetch: true } which forces the dedup to be skipped for the Query you give it to.

robermorales commented 5 years ago

In the meanwhile you can probably use context: { forceFetch: true } which forces the dedup to be skipped for the Query you give it to.

Thanks!!!

Druotic commented 5 years ago

@robermorales I'm not sure if you're referring specifically to the dedup link or if you are using the higher level client. If you are using apollo-link-dedup then https://github.com/apollographql/apollo-link/pull/957 may address your concerns, as well.

robermorales commented 5 years ago

I'm not sure if you're referring specifically to the dedup link or if you are using the higher level client. If you are using apollo-link-dedup then #957 may address your concerns, as well.

looks good, @Druotic