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

Passing Operations through HTTPNetworkTransportDelegate #1189

Closed
Kalvin126 opened this issue May 5, 2020 · 7 comments
Closed

Passing Operations through HTTPNetworkTransportDelegate #1189

Kalvin126 opened this issue May 5, 2020 · 7 comments
Labels
enhancement Issues outlining new things we want to do or things that will make our lives as devs easier networking-stack
Milestone

Comments

@Kalvin126
Copy link

Kalvin126 commented May 5, 2020

Hey Apollo Community! I'd like to post a feature request.

Use Case:
I would like to add headers to a URLRequest conditionally based on the Operation being committed.

Analysis:
I see that HTTPNetworkTransportPreflightDelegate has a function func networkTransport(_ networkTransport: HTTPNetworkTransport, willSend request: inout URLRequest). This is where it is intended to modify the URLRequest before it is actually sent out. However, there is no way to get the GraphQLOperation that is being committed. That delegate call is called in HTTPNetworkTransport's function:

func createRequest<Operation: GraphQLOperation>(for operation: Operation,
                                                isPersistedQueryRetry: Bool,
                                                files: [GraphQLFile]?) throws -> URLRequest

So a copy of the GraphQLOperation is held here. I was thinking that we could pass the operation with the delegate calls but the existence of an associated type in GraphQLOperation does not allow that.

The only way around that would be to move out the Data associated type, but that would break a bunch of other code and would require re-architecting. Another issue did talk about placing a context object. That could be a required member of GraphQLOperation that is then pass then passed to delegate instead of the Operation itself. I'd be interested in hearing the community's thoughts on this.

@designatednerd designatednerd added enhancement Issues outlining new things we want to do or things that will make our lives as devs easier networking-stack labels May 5, 2020
@designatednerd
Copy link
Contributor

Hey @Kalvin126 - it's a bit of a hack, but you can grab the JSON out of the httpBody of the request in willSend and grab the "operationName" in that json dict.

I'm in the middle of experimenting with a more interceptor-based system that will allow use cases like this a LOT more easily. I don't have an ETA but I think for now the workaround is probably preferable to adding another delegate method, particularly since there's somewhat limited need for accessing the operation at the point of adding headers.

Will leave this open to make sure I flag you when the network stuff is up.

@letko-dmitry
Copy link

btw, there is an other example: HTTPNetworkTransportGraphQLErrorDelegate. The callback receivedGraphQLErrors doesn't have any parameters allowing to identify and inspect nor operation or url request/response. I'd say it's extremely painful. Are there any plans to add more information like that to all the callbacks?

@designatednerd
Copy link
Contributor

Yes, a more interceptor-based system will allow passing more context and replace this delegate method.

@jamesbouker
Copy link

Hey, we recently added operation to

func networkTransport<T: GraphQLOperation>(_ networkTransport: HTTPNetworkTransport,
                                           operation: T,
                                           receivedError error: Error,
                                           for request: URLRequest,
                                           response: URLResponse?,
                                           retryHandler: @escaping (_ shouldRetry: Bool) -> Void)

Also extended GraphQLOperation to contain a retryCount: Int. Curious if anyone else has added a retryCount/limit and if so, how? Thanks!

@designatednerd
Copy link
Contributor

@Kalvin126 Please check out the RFC for networking changes in #1340 - that should help (and also includes a retryCount on the new request object).

@designatednerd
Copy link
Contributor

Hi, our new networking stack will be shipping as beta shortly and the ability to see the operation and request will be in there with it. I did change the retry count to be part of one of the interceptors after my previous comment. Please see #1341 for the details.

@designatednerd designatednerd added this to the Networking Beta milestone Sep 9, 2020
@designatednerd
Copy link
Contributor

The networking stack is now available as a beta at 0.33.0-beta1 (available via the betas/networking-stack branch and through PR #1386). Going to close this out, please open a new issue if your problems aren't addressed by the new networking stack. Thank you!

@designatednerd designatednerd modified the milestones: Networking Beta, Next Release Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues outlining new things we want to do or things that will make our lives as devs easier networking-stack
Projects
None yet
Development

No branches or pull requests

4 participants