Skip to content

Commit

Permalink
Merge pull request #770 from kimdv/kimdv/add-retry-if-graphql-errors
Browse files Browse the repository at this point in the history 
Introducing delegate if there is GraphQL errors
  • Loading branch information
@designatednerd
designatednerd authored Oct 29, 2019
2 parents d45f01b + 2e8d94a commit 0c56d45
Show file tree
Hide file tree
Showing 3 changed files with 163 additions and 12 deletions.
81 changes: 71 additions & 10 deletions Sources/Apollo/HTTPNetworkTransport.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,21 @@ public protocol HTTPNetworkTransportRetryDelegate: HTTPNetworkTransportDelegate
retryHandler: @escaping (_ shouldRetry: Bool) -> Void)
}

/// Methods which will be called after some kind of response has been received and it contains GraphQLErrors
public protocol HTTPNetworkTransportGraphQLErrorDelegate: HTTPNetworkTransportDelegate {


/// Called when response contains one or more GraphQL errors.
/// NOTE: Don't just call the `retryHandler` with `true` all the time, or you can potentially wind up in an infinite loop of errors
///
/// - Parameters:
/// - networkTransport: The network transport which received the error
/// - errors: The received GraphQL errors
/// - retryHandler: A closure indicating whether the operation should be retried. Asyncrhonous to allow for re-authentication or other async operations to complete.
func networkTransport(_ networkTransport: HTTPNetworkTransport, receivedGraphQLErrors errors: [GraphQLError], retryHandler: @escaping (_ shouldRetry: Bool) -> Void)
}


// MARK: -

/// A network transport that uses HTTP POST requests to send GraphQL operations to a server, and that uses `URLSession` as the networking implementation.
Expand Down Expand Up @@ -131,6 +146,7 @@ public class HTTPNetworkTransport {

if let receivedError = error {
self.handleErrorOrRetry(operation: operation,
files: files,
error: receivedError,
for: request,
response: response,
Expand All @@ -147,6 +163,7 @@ public class HTTPNetworkTransport {
response: httpResponse,
kind: .errorResponse)
self.handleErrorOrRetry(operation: operation,
files: files,
error: unsuccessfulError,
for: request,
response: response,
Expand All @@ -159,6 +176,7 @@ public class HTTPNetworkTransport {
response: httpResponse,
kind: .invalidResponse)
self.handleErrorOrRetry(operation: operation,
files: files,
error: error,
for: request,
response: response,
Expand All @@ -170,10 +188,13 @@ public class HTTPNetworkTransport {
guard let body = try self.serializationFormat.deserialize(data: data) as? JSONObject else {
throw GraphQLHTTPResponseError(body: data, response: httpResponse, kind: .invalidResponse)
}

let graphQLResponse = GraphQLResponse(operation: operation, body: body)

if let errors = graphQLResponse.parseErrorsOnlyFast() {
// Handle specific errors from response
self.handleGraphQLErrorsIfNeeded(operation: operation,
files: files,
for: request,
body: body,
errors: errors,
Expand All @@ -183,6 +204,7 @@ public class HTTPNetworkTransport {
}
} catch let parsingError {
self.handleErrorOrRetry(operation: operation,
files: files,
error: parsingError,
for: request,
response: response,
Expand All @@ -194,29 +216,60 @@ public class HTTPNetworkTransport {

return task
}

private func handleGraphQLErrorsOrComplete<Operation>(operation: Operation,
files: [GraphQLFile]?,
response: GraphQLResponse<Operation>,
completionHandler: @escaping (_ result: Result<GraphQLResponse<Operation>, Error>) -> Void) {
guard let delegate = self.delegate as? HTTPNetworkTransportGraphQLErrorDelegate,
let graphQLErrors = response.parseErrorsOnlyFast(),
!graphQLErrors.isEmpty else {
completionHandler(.success(response))
return
}

delegate.networkTransport(self, receivedGraphQLErrors: graphQLErrors, retryHandler: { [weak self] shouldRetry in
guard let self = self else {
// None of the rest of this really matters
return
}

guard shouldRetry else {
completionHandler(.success(response))
return
}

_ = self.send(operation: operation,
isPersistedQueryRetry: self.enableAutoPersistedQueries,
files: files,
completionHandler: completionHandler)
})
}

private func handleGraphQLErrorsIfNeeded<Operation>(operation: Operation,
files: [GraphQLFile]?,
for request: URLRequest,
body: JSONObject,
errors: [GraphQLError],
completionHandler: @escaping (_ results: Result<GraphQLResponse<Operation>, Error>) -> Void) {

let errorMessages = errors.compactMap { $0.message }
if self.enableAutoPersistedQueries,
errorMessages.contains("PersistedQueryNotFound") {
// We need to retry this with the full body.
_ = self.send(operation: operation,
isPersistedQueryRetry: true,
files: nil,
completionHandler: completionHandler)
// We need to retry this with the full body.
_ = self.send(operation: operation,
isPersistedQueryRetry: true,
files: nil,
completionHandler: completionHandler)
} else {
// Pass the response on to the rest of the chain
let response = GraphQLResponse(operation: operation, body: body)
completionHandler(.success(response))
handleGraphQLErrorsOrComplete(operation: operation, files: files, response: response, completionHandler: completionHandler)
}
}

private func handleErrorOrRetry<Operation>(operation: Operation,
files: [GraphQLFile]?,
error: Error,
for request: URLRequest,
response: URLResponse?,
Expand All @@ -234,12 +287,20 @@ public class HTTPNetworkTransport {
for: request,
response: response,
retryHandler: { [weak self] shouldRetry in
guard let self = self else {
// None of the rest of this really matters
return
}

guard shouldRetry else {
completionHandler(.failure(error))
return
}

_ = self?.send(operation: operation, completionHandler: completionHandler)
_ = self.send(operation: operation,
isPersistedQueryRetry: self.enableAutoPersistedQueries,
files: files,
completionHandler: completionHandler)
})
}

Expand Down Expand Up @@ -287,7 +348,7 @@ public class HTTPNetworkTransport {
sendQueryDocument: sendQueryDocument,
autoPersistQueries: autoPersistQueries)
}

private func createRequest<Operation: GraphQLOperation>(for operation: Operation,
files: [GraphQLFile]?,
httpMethod: GraphQLHTTPMethod,
Expand Down
10 changes: 9 additions & 1 deletion Tests/ApolloTestSupport/MockURLSession.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,27 @@ import Foundation

public final class MockURLSession: URLSession {
public private (set) var lastRequest: URLRequest?


public var data: Data?
public var response: HTTPURLResponse?
public var error: Error?

override public func dataTask(with request: URLRequest) -> URLSessionDataTask {
lastRequest = request
return URLSessionDataTaskMock()
}

override public func dataTask(with request: URLRequest, completionHandler: @escaping (Data?, URLResponse?, Error?) -> Void) -> URLSessionDataTask {
lastRequest = request
if let response = response {
completionHandler(data, response, error)
}
return URLSessionDataTaskMock()
}
}

private final class URLSessionDataTaskMock: URLSessionDataTask {
override func resume() {

}
}
84 changes: 83 additions & 1 deletion Tests/ApolloTests/HTTPTransportTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import XCTest
@testable import Apollo
import StarWarsAPI
import ApolloTestSupport

class HTTPTransportTests: XCTestCase {

Expand All @@ -23,6 +24,8 @@ class HTTPTransportTests: XCTestCase {
private var shouldModifyURLInWillSend = false
private var retryCount = 0

private var graphQlErrors = [GraphQLError]()

private lazy var url = URL(string: "http://localhost:8080/graphql")!
private lazy var networkTransport = HTTPNetworkTransport(url: self.url,
useGETForQueries: true,
Expand Down Expand Up @@ -59,7 +62,7 @@ class HTTPTransportTests: XCTestCase {
line: line)
}
}

func testPreflightDelegateTellingRequestNotToSend() {
self.shouldSend = false

Expand Down Expand Up @@ -193,6 +196,74 @@ class HTTPTransportTests: XCTestCase {
delegate: self)
XCTAssertNotEqual(self.networkTransport, nonIdenticalTransport)
}

func testErrorDelegateWithErrors() throws {
self.retryCount = 0
self.graphQlErrors = []
let query = HeroNameQuery()
// TODO: Replace this with once it is codable https://github.com/apollographql/apollo-ios/issues/467
let body = ["errors": [["message": "Test graphql error"]]]

let mockSession = MockURLSession()
mockSession.response = HTTPURLResponse(url: url, statusCode: 200, httpVersion: nil, headerFields: nil)
mockSession.data = try JSONSerialization.data(withJSONObject: body, options: .prettyPrinted)
let network = HTTPNetworkTransport(url: url, session: mockSession, delegate: self)
let expectation = self.expectation(description: "Send operation completed")

let _ = network.send(operation: query) { result in
switch result {
case .success:
expectation.fulfill()
case .failure:
break
}
}

guard let request = mockSession.lastRequest else {
XCTFail("last request should not be nil")
return
}
XCTAssertEqual(request.url?.host, network.url.host)
XCTAssertEqual(request.httpMethod, "POST")

XCTAssertEqual(self.graphQlErrors.count, 1)
XCTAssertEqual(retryCount, 1)
wait(for: [expectation], timeout: 1)
}

func testErrorDelegateWithNoErrors() throws {
self.retryCount = 0
self.graphQlErrors = []
let query = HeroNameQuery()
// TODO: Replace this with once it is codable https://github.com/apollographql/apollo-ios/issues/467
let body = ["errors": []]

let mockSession = MockURLSession()
mockSession.response = HTTPURLResponse(url: url, statusCode: 200, httpVersion: nil, headerFields: nil)
mockSession.data = try JSONSerialization.data(withJSONObject: body, options: .prettyPrinted)
let network = HTTPNetworkTransport(url: url, session: mockSession, delegate: self)
let expectation = self.expectation(description: "Send operation completed")

let _ = network.send(operation: query) { result in
switch result {
case .success:
expectation.fulfill()
case .failure:
break
}
}

guard let request = mockSession.lastRequest else {
XCTFail("last request should not be nil")
return
}
XCTAssertEqual(request.url?.host, network.url.host)
XCTAssertEqual(request.httpMethod, "POST")
XCTAssertEqual(self.retryCount, 0)
XCTAssertEqual(self.graphQlErrors.count, 0)
wait(for: [expectation], timeout: 1)

}
}

// MARK: - HTTPNetworkTransportPreflightDelegate
Expand Down Expand Up @@ -266,3 +337,14 @@ extension HTTPTransportTests: HTTPNetworkTransportRetryDelegate {
}
}
}

// MARK: - HTTPNetworkTransportGraphQLErrorDelegate

extension HTTPTransportTests: HTTPNetworkTransportGraphQLErrorDelegate {
func networkTransport(_ networkTransport: HTTPNetworkTransport, receivedGraphQLErrors errors: [GraphQLError], retryHandler: @escaping (Bool) -> Void) {
self.retryCount += 1
let shouldRetry = retryCount == 2
self.graphQlErrors = errors
retryHandler(shouldRetry)
}
}

0 comments on commit 0c56d45

Please sign in to comment.