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

Introducing delegate if there is GraphQL errors #770

Merged
81 changes: 71 additions & 10 deletions Sources/Apollo/HTTPNetworkTransport.swift
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,
kimdv marked this conversation as resolved.
Show resolved Hide resolved
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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

this is more of a note to self than to you: I need to add my readability isNotEmpty boolean extension to this code 🙃

Copy link
Contributor

Choose a reason for hiding this comment

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

Done in #872

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
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
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"]]]
designatednerd marked this conversation as resolved.
Show resolved Hide resolved

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
kimdv marked this conversation as resolved.
Show resolved Hide resolved
retryHandler(shouldRetry)
}
}