Skip to content

Commit af31899

Browse files
authored
Prevent multiple continuation resume calls (#196)
<!-- 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 -->
1 parent 361794f commit af31899

File tree

5 files changed

+188
-119
lines changed

5 files changed

+188
-119
lines changed

Sources/ForageSDK/Foundation/Network/Provider.swift

Lines changed: 13 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -45,77 +45,29 @@ class Provider {
4545
}
4646
}
4747

48-
func execute(endpoint: ServiceProtocol, completion: @escaping (Result<Data?, Error>) -> Void) throws {
49-
do {
50-
let request = try endpoint.urlRequest()
51-
task = urlSession.dataTask(with: request) { data, response, error in
52-
self.processResponse(response: response) { result in
53-
DispatchQueue.main.async {
54-
switch result {
55-
case .success:
56-
completion(.success(data))
57-
case .failure:
58-
if let data = data {
59-
self.processVaultData(
60-
model: ForageServiceError.self,
61-
code: nil,
62-
data: data,
63-
response: response
64-
) { errorResult in
65-
switch errorResult {
66-
case let .success(errorParsed):
67-
return completion(.failure(errorParsed))
68-
case let .failure(error):
69-
return completion(.failure(error))
70-
}
71-
}
72-
} else if let error = error {
73-
return completion(.failure(error))
74-
} else {
75-
return completion(.failure(ServiceError.emptyError))
76-
}
77-
}
78-
}
79-
}
80-
}
81-
task?.resume()
82-
} catch {
83-
throw error
84-
}
85-
}
86-
8748
func stopRequestOnGoing() {
8849
if let task = task {
8950
task.cancel()
9051
}
9152
}
9253

9354
private func middleware<T: Decodable>(model: T.Type, data: Data?, response: URLResponse?, error: Error?, completion: @escaping (Result<T, Error>) -> Void) {
94-
var httpResponse: HTTPURLResponse?
95-
96-
processResponse(response: response) { result in
97-
switch result {
98-
case let .success(response):
99-
self.logger?.info(
100-
"Received \(response.statusCode) response from \(self.getResponseUrlPath(response))",
101-
attributes: ["endpoint": httpResponse?.url?.path]
102-
)
103-
httpResponse = response
104-
case let .failure(error):
105-
self.logger?.error("Failed to process response", error: error, attributes: nil)
106-
return completion(.failure(error))
107-
}
55+
if let error = error {
56+
let httpResponse = response as? HTTPURLResponse
57+
let wrappedError = NSError(domain: "Error: \(error)", code: httpResponse?.statusCode ?? 500, userInfo: nil)
58+
self.logger?.error("Failed to process error for \(self.getResponseUrlPath(httpResponse))", error: wrappedError, attributes: nil)
59+
return completion(.failure(wrappedError))
10860
}
10961

110-
processError(error: error, response: httpResponse) { result in
111-
switch result {
112-
case .success:
113-
break
114-
case let .failure(error):
115-
self.logger?.error("Failed to process error for \(self.getResponseUrlPath(httpResponse))", error: error, attributes: nil)
116-
completion(.failure(error))
117-
}
62+
guard let httpResponse = response as? HTTPURLResponse else {
63+
let wrappedError = CommonErrors.UNKNOWN_SERVER_ERROR
64+
self.logger?.error("Failed to process response", error: wrappedError, attributes: nil)
65+
return completion(.failure(wrappedError))
11866
}
67+
self.logger?.info(
68+
"Received \(httpResponse.statusCode) response from \(self.getResponseUrlPath(httpResponse))",
69+
attributes: ["endpoint": httpResponse.url?.path]
70+
)
11971

12072
processData(model: model, data: data, response: httpResponse, error: error) { result in
12173
switch result {
@@ -128,22 +80,6 @@ class Provider {
12880
}
12981
}
13082

131-
private func processResponse(response: URLResponse?, completion: @escaping (Result<HTTPURLResponse, Error>) -> Void) {
132-
guard let httpResponse = response as? HTTPURLResponse else {
133-
return completion(.failure(CommonErrors.UNKNOWN_SERVER_ERROR))
134-
}
135-
136-
return completion(.success(httpResponse))
137-
}
138-
139-
private func processError(error: Error?, response: HTTPURLResponse?, completion: @escaping (Result<Void, Error>) -> Void) {
140-
guard let error = error else {
141-
return completion(.success(()))
142-
}
143-
144-
completion(.failure(NSError(domain: "Error: \(error)", code: response?.statusCode ?? 500, userInfo: nil)))
145-
}
146-
14783
private func processData<T: Decodable>(model: T.Type, data: Data?, response: HTTPURLResponse?, error: Error?, completion: @escaping (Result<T, Error>) -> Void) {
14884
guard let data = data else {
14985
return completion(.failure(ForageError.create(
@@ -181,45 +117,4 @@ class Provider {
181117
return "\(host)\(path)"
182118
}
183119

184-
func processVaultData<T: Decodable>(model: T.Type, code: Int?, data: Data?, response: URLResponse?, completion: @escaping (Result<T, Error>) -> Void) {
185-
var httpResponse: HTTPURLResponse?
186-
187-
processResponse(response: response) { result in
188-
switch result {
189-
case let .success(response):
190-
httpResponse = response
191-
case let .failure(error):
192-
return completion(.failure(error))
193-
}
194-
}
195-
196-
guard let data = data else {
197-
return completion(.failure(ForageError.create(
198-
code: "invalid_input_data",
199-
httpStatusCode: httpResponse?.statusCode ?? 500,
200-
message: "Double check the reference documentation to validate the request body, and scan your implementation for any other errors."
201-
)))
202-
}
203-
204-
guard let result = try? JSONDecoder().decode(T.self, from: data) else {
205-
// NOW TRY TO DECODE IT AS AN ERROR!
206-
guard let forageServiceError = try? JSONDecoder().decode(ForageServiceError.self, from: data) else {
207-
return completion(.failure(ForageError.create(
208-
code: "unknown_server_error",
209-
httpStatusCode: httpResponse?.statusCode ?? 500,
210-
message: "Could not decode payload - \(String(decoding: data, as: UTF8.self))"
211-
)))
212-
}
213-
214-
let code = forageServiceError.errors[0].code
215-
let message = forageServiceError.errors[0].message
216-
return completion(.failure(ForageError.create(
217-
code: code,
218-
httpStatusCode: httpResponse?.statusCode ?? 500,
219-
message: message
220-
)))
221-
}
222-
223-
return completion(.success(result))
224-
}
225120
}

Tests/ForageSDKTests/ForagePublicSubmitMethodTests.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ final class ForagePublicSubmitMethodTests: XCTestCase {
136136
mockService.doesCheckBalanceThrow = doesThrow
137137
let mockPinTextField = createMockPinTextField(isComplete: pinComplete)
138138
let expectation = XCTestExpectation(description: description)
139+
expectation.assertForOverFulfill = true
139140

140141
MockForageSDK.shared.checkBalance(
141142
foragePinTextField: mockPinTextField,
@@ -158,6 +159,7 @@ final class ForagePublicSubmitMethodTests: XCTestCase {
158159
mockService.doesCapturePaymentThrow = doesThrow
159160
let mockPinTextField = createMockPinTextField(isComplete: pinComplete)
160161
let expectation = XCTestExpectation(description: description)
162+
expectation.assertForOverFulfill = true
161163

162164
MockForageSDK.shared.capturePayment(
163165
foragePinTextField: mockPinTextField,
@@ -179,6 +181,7 @@ final class ForagePublicSubmitMethodTests: XCTestCase {
179181
mockService.doesCollectPinThrow = doesThrow
180182
let mockPinTextField = createMockPinTextField(isComplete: pinComplete)
181183
let expectation = XCTestExpectation(description: description)
184+
expectation.assertForOverFulfill = true
182185

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

196199
func testTokenizeEBTCard_Success() {
197200
let expectation = XCTestExpectation(description: "Returns PaymentMethod response")
201+
expectation.assertForOverFulfill = true
198202
let mockPanTextField = ForagePANTextField(frame: .zero)
199203

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

222226
func testTokenizeEBTCard_Throws_DoesRejectWithError() {
223227
let expectation = XCTestExpectation(description: "tokenizeEBTCard rejects with ForageError")
228+
expectation.assertForOverFulfill = true
224229
let mockPanTextField = ForagePANTextField(frame: .zero)
225230

226231
(MockForageSDK.shared.service as! MockForageService).doesTokenizeEBTCardThrow = true

0 commit comments

Comments
 (0)