-
Notifications
You must be signed in to change notification settings - Fork 737
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
A per request/response context object passed among delegate callbacks #1132
Comments
I'm gonna think this through - I think this might be more work than is worth it if the headers are the one thing everyone actually needs access to. I know the |
It would be incredible of this context also included away to get at the NSProgress of the underlying network request. Have been thinking about how best to expose this myself, was considering a similar pattern but I've got no idea what happens/how to handle when the response is cached or optimistically updated. |
For transparency's sake: I'm looking at a wholesale revamp of our HTTP request system to get something a bit more in line with the OKHTTP-inspired interceptors in our Android SDK and/or ApolloLink in our web SDK. Basically, something where you can create a series of objects which do work for you and add your own arbitrary ones in as you want - this will remove the need for the gazillion delegate callbacks and give information at each stage of the process about the request and its associated response. Still at the noodling phase but realistically this needs to be done anyway because the delegate situation is getting pretty out of control, and we also need to make some significant changes to allow use of background URL sessions. I'm hoping to post a call for feedback in the next week or two - obviously trying to do two things at the same time with the codegen thing is a pain, but from the issues being raised here it does seem like this could have a big impact (and ultimately make it easier to use old vs new codegen, I hope). |
Please check out the networking update RFC #1340 @xma000 @sneakyness on this - I'm going to close this out when the network rewrite ships but I believe that should solve the issues you're encountering here. |
Hi, our new networking stack will be shipping as a beta soon and I believe it should solve the issues you've encountered here. Please see #1341 for the current implementation. |
The networking stack is now available as a beta at |
We have a use case where we want to process HTTP headers and/or status code along with the GraphQL errors for a response. The callback for each of
HTTPNetworkTransportTaskCompletedDelegate
andHTTPNetworkTransportGraphQLErrorDelegate
only contains information belonging to that layer, which is the right design. However, that means I won't receive information from different layers in one callback.I propose that we add a per request context object to
HTTPNetworkTransport
. This mutable context object can hold attribute objects identified by string keys. This context object will be passed in every delegate callback for the request/response. This is a common technique to allow processors in multiple layers to coordinate. I think this is a great enhancement to the client library and is not difficult to implement.With this support, we can set the header value to the context when we receive the callback through the interface of
HTTPNetworkTransportTaskCompletedDelegate
, then retrieve this value from the context when we process the callback forHTTPNetworkTransportGraphQLErrorDelegate
.This issue is a follow up to #1116.
The text was updated successfully, but these errors were encountered: