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.

Sign up for GitHub

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

Jump to bottom

Network Rearchitecture #1341

Merged
merged 109 commits into from
Sep 11, 2020
Merged

Network Rearchitecture #1341

merged 109 commits into from
Sep 11, 2020

Conversation

designatednerd
Copy link
Contributor

This PR relates to the RFC for the new networking stack - this branch is basically where I've been doing explorations thus far.

@designatednerd designatednerd mentioned this pull request Jul 31, 2020
Closed
@designatednerd designatednerd force-pushed the explore/interceptors branch 2 times, most recently from 6172015 to df6024d Compare August 4, 2020 21:30
@designatednerd designatednerd mentioned this pull request Aug 6, 2020
Merged
@designatednerd designatednerd force-pushed the explore/interceptors branch 4 times, most recently from 91c9c47 to 0bde055 Compare August 13, 2020 01:51
@designatednerd designatednerd force-pushed the explore/interceptors branch 2 times, most recently from 6af46dc to 515b1db Compare August 20, 2020 19:39
@designatednerd designatednerd mentioned this pull request Aug 20, 2020
Closed
amadeu01
amadeu01 reviewed Aug 26, 2020
View reviewed changes
Sources/Apollo/ApolloErrorInterceptor.swift
import Foundation

/// An error interceptor called to allow further examination of error data when an error occurs in the chain.
public protocol ApolloErrorInterceptor {
Copy link

@amadeu01 amadeu01 Aug 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it would work, but what do you think of instead using protocol for this interceptor, actually, use just a function. For instance,

typealias ApolloErrorInterceptor =  (Error,
       RequestChain,
       HTTPRequest<Operation>,
       HTTPResponse<Operation>?,
       @escaping (Result<GraphQLResult<Operation.Data>, Error>) -> Void)) -> Void

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would wayyyyyy rather have this as a protocol, particularly since it gives longer term flexibility to add more methods if needed.

@designatednerd designatednerd mentioned this pull request Aug 31, 2020
Merged
@designatednerd designatednerd force-pushed the explore/interceptors branch 4 times, most recently from af574ef to 1dd71de Compare September 3, 2020 18:04
designatednerd
designatednerd commented Sep 3, 2020
View reviewed changes
Sources/Apollo/GraphQLResult.swift
extension GraphQLResult where Data: Decodable {

public init<T: FlexibleDecoder>(from data: Foundation.Data, decoder: T) throws {
// SWIFT CODEGEN: fix this to handle codable better
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoroughly punting on this for now.

@designatednerd
Copy link
Contributor Author

A preview of the updated networking setup instructions can be found here.

Going to be updating the tutorial portion dealing with these changes before cutting a beta. Would love feedback!

@designatednerd designatednerd changed the title [DRAFT]: Initial stab at network rearchitecture [DRAFT]: Network Rearchitecture Sep 9, 2020
This was referenced Sep 9, 2020
Closed
Closed
Closed
Closed
Closed
@designatednerd
Use a UUID for the task rather than an unsafe mutable raw pointer to …
4abe114 
…dedupe callbacks for the watcher.
@designatednerd
boop fetch expectation time back down
e9879f2
@designatednerd
add now-required parameters to web socket
c295cd9
@designatednerd
fix typo 🤦‍♀️
c6a1f99
@designatednerd
use context identifier rather than task identifier to disambiguate id…
e363582 
…entifier for watcher context vs identifier for actual URLSessionTask
@designatednerd
Fix missing documentation
fdaf7b4
@designatednerd
rename identifier to context identifier in callback, add docs to apol…
80927a2 
…lo store subscriber for future reference
@designatednerd
fix missed contextIdentifer
ba76ace
@designatednerd
align cache miss on .returnCacheDataAndFetch with existing code
9516ab2
@designatednerd
rm whitespace
e588a52
@designatednerd
add context identifier to equality comparison on HTTPRequest
624df57
@designatednerd
Remove the retry count from the request and make it a property of the…
4bd0d6a 
… retry interceptor
@designatednerd
documentation for Parseable
caead65
@designatednerd
make sure errors are public and localized
2a6df16
@designatednerd
don't use an apollo store in the codable transport (yet?)
e220b02
@designatednerd
Make sure Foundation is imported where needed
b09ffcb
@designatednerd
make sure foundation is imported on SplitNetworkTransport
795af72
@designatednerd
move setup for additional headers into the initializer of HTTPRequest
bc42ea5
@designatednerd
add debug description for http request
d18aab3
@designatednerd
completely rewrite the Initialization document to handle setting up…
f35ae0a 
… with request chain
@designatednerd
Make provided interceptor providers open so they can be subclassed.
3f2a6e7
@designatednerd
update tutorial for request chain network transport
38135ee
@designatednerd designatednerd changed the base branch from main to betas/networking-stack September 10, 2020 21:36
@designatednerd designatednerd changed the title [DRAFT]: Network Rearchitecture Network Rearchitecture Sep 10, 2020
@designatednerd designatednerd marked this pull request as ready for review September 10, 2020 21:36
@designatednerd
Copy link
Contributor Author

OK, pointing this at betas/networking-stack - once tests pass here I'll be cutting a beta from that branch and further PRs will go to that branch until the networking stack gets merged to main.

@designatednerd designatednerd merged commit 713430d into betas/networking-stack Sep 11, 2020
@designatednerd designatednerd deleted the explore/interceptors branch September 11, 2020 18:19
@benjaminsaurusrex benjaminsaurusrex mentioned this pull request Apr 13, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amadeu01 amadeu01 amadeu01 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@designatednerd @amadeu01