Skip to content

Commit

Permalink
Prevent multiple continuation resume calls (#196)
Browse files Browse the repository at this point in the history
<!-- Update your title to prefix with your ticket number -->

## What
<!-- Describe your changes here -->
Credit to [cltnschlosser](https://github.com/cltnschlosser) for this PR.

Update tests and `Provider.swift` to prevent multiple continuation resume calls. Also removed unused code to reduce maintenance burden.


<!-- If you are making a front-end change, please include a screen recording and post it in #feature-recordings -->

## Why
Multiple calls to resume a continuation caused runtime exceptions, which should not happen
<!-- Describe the motivations behind this change if they are a subset of your ticket -->

## Test Plan
Updated relevant tests to assert that continuation resume is never called more than once. Also added a few other test cases.
<!-- IMPORTANT: QA Tests and Unit Tests must be passed locally before this PR can be merged. -->

- ✅  Not a front-end change
- ❌ iOS QA Tests passed locally - I didn't run them locally but the QA test check passes
- ✅  Unit Tests passed locally - yes, and the coverage check runs the tests and they pass

## How
Can be merged right away
<!-- Describe the rollout plan if it includes multiple PRs/Repos or requires extra steps beyond rolling back the Service -->
  • Loading branch information
devinmorgan authored Dec 13, 2024
1 parent 361794f commit af31899
Show file tree
Hide file tree
Showing 5 changed files with 188 additions and 119 deletions.
131 changes: 13 additions & 118 deletions Sources/ForageSDK/Foundation/Network/Provider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,77 +45,29 @@ class Provider {
}
}

func execute(endpoint: ServiceProtocol, completion: @escaping (Result<Data?, Error>) -> Void) throws {
do {
let request = try endpoint.urlRequest()
task = urlSession.dataTask(with: request) { data, response, error in
self.processResponse(response: response) { result in
DispatchQueue.main.async {
switch result {
case .success:
completion(.success(data))
case .failure:
if let data = data {
self.processVaultData(
model: ForageServiceError.self,
code: nil,
data: data,
response: response
) { errorResult in
switch errorResult {
case let .success(errorParsed):
return completion(.failure(errorParsed))
case let .failure(error):
return completion(.failure(error))
}
}
} else if let error = error {
return completion(.failure(error))
} else {
return completion(.failure(ServiceError.emptyError))
}
}
}
}
}
task?.resume()
} catch {
throw error
}
}

func stopRequestOnGoing() {
if let task = task {
task.cancel()
}
}

private func middleware<T: Decodable>(model: T.Type, data: Data?, response: URLResponse?, error: Error?, completion: @escaping (Result<T, Error>) -> Void) {
var httpResponse: HTTPURLResponse?

processResponse(response: response) { result in
switch result {
case let .success(response):
self.logger?.info(
"Received \(response.statusCode) response from \(self.getResponseUrlPath(response))",
attributes: ["endpoint": httpResponse?.url?.path]
)
httpResponse = response
case let .failure(error):
self.logger?.error("Failed to process response", error: error, attributes: nil)
return completion(.failure(error))
}
if let error = error {
let httpResponse = response as? HTTPURLResponse
let wrappedError = NSError(domain: "Error: \(error)", code: httpResponse?.statusCode ?? 500, userInfo: nil)
self.logger?.error("Failed to process error for \(self.getResponseUrlPath(httpResponse))", error: wrappedError, attributes: nil)
return completion(.failure(wrappedError))
}

processError(error: error, response: httpResponse) { result in
switch result {
case .success:
break
case let .failure(error):
self.logger?.error("Failed to process error for \(self.getResponseUrlPath(httpResponse))", error: error, attributes: nil)
completion(.failure(error))
}
guard let httpResponse = response as? HTTPURLResponse else {
let wrappedError = CommonErrors.UNKNOWN_SERVER_ERROR
self.logger?.error("Failed to process response", error: wrappedError, attributes: nil)
return completion(.failure(wrappedError))
}
self.logger?.info(
"Received \(httpResponse.statusCode) response from \(self.getResponseUrlPath(httpResponse))",
attributes: ["endpoint": httpResponse.url?.path]
)

processData(model: model, data: data, response: httpResponse, error: error) { result in
switch result {
Expand All @@ -128,22 +80,6 @@ class Provider {
}
}

private func processResponse(response: URLResponse?, completion: @escaping (Result<HTTPURLResponse, Error>) -> Void) {
guard let httpResponse = response as? HTTPURLResponse else {
return completion(.failure(CommonErrors.UNKNOWN_SERVER_ERROR))
}

return completion(.success(httpResponse))
}

private func processError(error: Error?, response: HTTPURLResponse?, completion: @escaping (Result<Void, Error>) -> Void) {
guard let error = error else {
return completion(.success(()))
}

completion(.failure(NSError(domain: "Error: \(error)", code: response?.statusCode ?? 500, userInfo: nil)))
}

private func processData<T: Decodable>(model: T.Type, data: Data?, response: HTTPURLResponse?, error: Error?, completion: @escaping (Result<T, Error>) -> Void) {
guard let data = data else {
return completion(.failure(ForageError.create(
Expand Down Expand Up @@ -181,45 +117,4 @@ class Provider {
return "\(host)\(path)"
}

func processVaultData<T: Decodable>(model: T.Type, code: Int?, data: Data?, response: URLResponse?, completion: @escaping (Result<T, Error>) -> Void) {
var httpResponse: HTTPURLResponse?

processResponse(response: response) { result in
switch result {
case let .success(response):
httpResponse = response
case let .failure(error):
return completion(.failure(error))
}
}

guard let data = data else {
return completion(.failure(ForageError.create(
code: "invalid_input_data",
httpStatusCode: httpResponse?.statusCode ?? 500,
message: "Double check the reference documentation to validate the request body, and scan your implementation for any other errors."
)))
}

guard let result = try? JSONDecoder().decode(T.self, from: data) else {
// NOW TRY TO DECODE IT AS AN ERROR!
guard let forageServiceError = try? JSONDecoder().decode(ForageServiceError.self, from: data) else {
return completion(.failure(ForageError.create(
code: "unknown_server_error",
httpStatusCode: httpResponse?.statusCode ?? 500,
message: "Could not decode payload - \(String(decoding: data, as: UTF8.self))"
)))
}

let code = forageServiceError.errors[0].code
let message = forageServiceError.errors[0].message
return completion(.failure(ForageError.create(
code: code,
httpStatusCode: httpResponse?.statusCode ?? 500,
message: message
)))
}

return completion(.success(result))
}
}
5 changes: 5 additions & 0 deletions Tests/ForageSDKTests/ForagePublicSubmitMethodTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ final class ForagePublicSubmitMethodTests: XCTestCase {
mockService.doesCheckBalanceThrow = doesThrow
let mockPinTextField = createMockPinTextField(isComplete: pinComplete)
let expectation = XCTestExpectation(description: description)
expectation.assertForOverFulfill = true

MockForageSDK.shared.checkBalance(
foragePinTextField: mockPinTextField,
Expand All @@ -158,6 +159,7 @@ final class ForagePublicSubmitMethodTests: XCTestCase {
mockService.doesCapturePaymentThrow = doesThrow
let mockPinTextField = createMockPinTextField(isComplete: pinComplete)
let expectation = XCTestExpectation(description: description)
expectation.assertForOverFulfill = true

MockForageSDK.shared.capturePayment(
foragePinTextField: mockPinTextField,
Expand All @@ -179,6 +181,7 @@ final class ForagePublicSubmitMethodTests: XCTestCase {
mockService.doesCollectPinThrow = doesThrow
let mockPinTextField = createMockPinTextField(isComplete: pinComplete)
let expectation = XCTestExpectation(description: description)
expectation.assertForOverFulfill = true

MockForageSDK.shared.deferPaymentCapture(
foragePinTextField: mockPinTextField,
Expand All @@ -195,6 +198,7 @@ final class ForagePublicSubmitMethodTests: XCTestCase {

func testTokenizeEBTCard_Success() {
let expectation = XCTestExpectation(description: "Returns PaymentMethod response")
expectation.assertForOverFulfill = true
let mockPanTextField = ForagePANTextField(frame: .zero)

MockForageSDK.shared.tokenizeEBTCard(
Expand All @@ -221,6 +225,7 @@ final class ForagePublicSubmitMethodTests: XCTestCase {

func testTokenizeEBTCard_Throws_DoesRejectWithError() {
let expectation = XCTestExpectation(description: "tokenizeEBTCard rejects with ForageError")
expectation.assertForOverFulfill = true
let mockPanTextField = ForagePANTextField(frame: .zero)

(MockForageSDK.shared.service as! MockForageService).doesTokenizeEBTCardThrow = true
Expand Down
Loading

0 comments on commit af31899

Please sign in to comment.