From efede6d9f9594cbd94a840547014ed5fe560752d Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 16 Sep 2024 14:01:14 +0100 Subject: [PATCH 1/5] Log stripe error codes within the CardReaderService --- .../StripeCardReaderService.swift | 56 ++++++++++++------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/Hardware/Hardware/CardReader/StripeCardReader/StripeCardReaderService.swift b/Hardware/Hardware/CardReader/StripeCardReader/StripeCardReaderService.swift index 6a6c2e2e597..8c5735c242b 100644 --- a/Hardware/Hardware/CardReader/StripeCardReader/StripeCardReaderService.swift +++ b/Hardware/Hardware/CardReader/StripeCardReader/StripeCardReaderService.swift @@ -235,9 +235,8 @@ extension StripeCardReaderService: CardReaderService { return promise(.success(())) } - self?.internalError(error) + let underlyingError = Self.logAndDecodeError(error) discoveryLock.unlock() - let underlyingError = UnderlyingError(with: error) promise(.failure(CardReaderServiceError.discovery(underlyingError: underlyingError))) } } @@ -245,7 +244,7 @@ extension StripeCardReaderService: CardReaderService { } public func disconnect() -> Future { - return Future() { promise in + return Future() { [weak self] promise in // Throw an error if the SDK has not been initialized. // This prevent a crash when logging out or switching stores before // the SDK has been initialized. @@ -271,12 +270,12 @@ extension StripeCardReaderService: CardReaderService { Terminal.shared.disconnectReader { error in DispatchQueue.main.asyncAfter(deadline: .now() + 1) { if let error = error { - let underlyingError = UnderlyingError(with: error) + let underlyingError = Self.logAndDecodeError(error) promise(.failure(CardReaderServiceError.disconnection(underlyingError: underlyingError))) } if error == nil { - self.connectedReadersSubject.send([]) + self?.connectedReadersSubject.send([]) promise(.success(())) } } @@ -407,7 +406,7 @@ extension StripeCardReaderService: CardReaderService { let cancelPaymentIntent = { [weak self] in Terminal.shared.cancelPaymentIntent(activePaymentIntent) { (intent, error) in if let error = error { - let underlyingError = UnderlyingError(with: error) + let underlyingError = Self.logAndDecodeError(error) promise(.failure(CardReaderServiceError.paymentCancellation(underlyingError: underlyingError))) } @@ -473,7 +472,7 @@ extension StripeCardReaderService: CardReaderService { let config = try buildConfig.build() return promise(.success(config)) } catch { - let underlyingError = UnderlyingError(with: error) + let underlyingError = Self.logAndDecodeError(error) return promise(.failure(CardReaderServiceError.connection(underlyingError: underlyingError))) } case .failure(let error): @@ -505,7 +504,7 @@ extension StripeCardReaderService: CardReaderService { let config = try localMobileConfig.build() return promise(.success(config)) } catch { - let underlyingError = UnderlyingError(with: error) + let underlyingError = Self.logAndDecodeError(error) return promise(.failure(CardReaderServiceError.connection(underlyingError: underlyingError))) } case .failure(let error): @@ -537,7 +536,7 @@ extension StripeCardReaderService: CardReaderService { self.discoveredStripeReadersCache.clear() if let error = error { - let underlyingError = UnderlyingError(with: error) + let underlyingError = Self.logAndDecodeError(error) // Starting with StripeTerminal 2.0, required software updates happen transparently on connection // Any error related to that will be reported here, but we don't want to treat it as a connection error let serviceError: CardReaderServiceError = underlyingError.isSoftwareUpdateError ? @@ -577,7 +576,7 @@ extension StripeCardReaderService: CardReaderService { self.discoveredStripeReadersCache.clear() if let error = error { - let underlyingError = UnderlyingError(with: error) + let underlyingError = Self.logAndDecodeError(error) // Starting with StripeTerminal 2.0, required software updates happen transparently on connection // Any error related to that will be reported here, but we don't want to treat it as a connection error let serviceError: CardReaderServiceError = underlyingError.isSoftwareUpdateError ? @@ -641,7 +640,7 @@ private extension StripeCardReaderService { Terminal.shared.createPaymentIntent(parameters) { (intent, error) in if let error = error { - let underlyingError = UnderlyingError(with: error) + let underlyingError = Self.logAndDecodeError(error) promise(.failure(CardReaderServiceError.intentCreation(underlyingError: underlyingError))) } @@ -661,7 +660,7 @@ private extension StripeCardReaderService { /// to this cancellable if we want to cancel self?.paymentCancellable = Terminal.shared.collectPaymentMethod(intent) { (intent, error) in if let error = error { - var underlyingError = UnderlyingError(with: error) + var underlyingError = Self.logAndDecodeError(error) /// the completion block for collectPaymentMethod will be called /// with error Canceled when collectPaymentMethod is canceled /// https://stripe.dev/stripe-terminal-ios/docs/Classes/SCPTerminal.html#/c:objc(cs)SCPTerminal(im)collectPaymentMethod:delegate:completion: @@ -695,7 +694,7 @@ private extension StripeCardReaderService { Terminal.shared.confirmPaymentIntent(intent) { (intent, error) in guard let self = self else { return } if let error = error { - let underlyingError = UnderlyingError(with: error) + let underlyingError = Self.logAndDecodeError(error) guard let paymentIntent = error.paymentIntent else { return promise(.failure(CardReaderServiceError.paymentCapture(underlyingError: underlyingError))) @@ -766,8 +765,9 @@ extension StripeCardReaderService { self?.refundCancellable = Terminal.shared.collectRefundPaymentMethod(parameters) { [weak self] collectError in if let error = collectError { self?.refundCancellable = nil + let underlyingError = Self.logAndDecodeError(error) promise(.failure(CardReaderServiceError.refundPayment( - underlyingError: UnderlyingError(with: error), + underlyingError: underlyingError, shouldRetry: true ))) } else { @@ -834,7 +834,8 @@ extension StripeCardReaderService { refundCancellable.cancel({ error in if let error = error { - promise(.failure(CardReaderServiceError.refundCancellation(underlyingError: UnderlyingError(with: error)))) + let underlyingError = Self.logAndDecodeError(error) + promise(.failure(CardReaderServiceError.refundCancellation(underlyingError: underlyingError))) } promise(.success(())) }) @@ -874,8 +875,9 @@ extension StripeCardReaderService: BluetoothReaderDelegate { public func reader(_ reader: Reader, didFinishInstallingUpdate update: ReaderSoftwareUpdate?, error: Error?) { if let error = error { + let underlyingError = Self.logAndDecodeError(error) softwareUpdateSubject.send(.failed( - error: CardReaderServiceError.softwareUpdate(underlyingError: UnderlyingError(with: error), + error: CardReaderServiceError.softwareUpdate(underlyingError: underlyingError, batteryLevel: reader.batteryLevel?.doubleValue)) ) if let requiredDate = update?.requiredAt, @@ -966,8 +968,9 @@ extension StripeCardReaderService: LocalMobileReaderDelegate { public func localMobileReader(_ reader: Reader, didFinishInstallingUpdate update: ReaderSoftwareUpdate?, error: Error?) { if let error = error { + let underlyingError = Self.logAndDecodeError(error) softwareUpdateSubject.send(.failed( - error: CardReaderServiceError.softwareUpdate(underlyingError: UnderlyingError(with: error), + error: CardReaderServiceError.softwareUpdate(underlyingError: underlyingError, batteryLevel: reader.batteryLevel?.doubleValue)) ) if let requiredDate = update?.requiredAt, @@ -1011,7 +1014,7 @@ private extension StripeCardReaderService { func resetDiscoveredReadersSubject(error: Error? = nil) { if let error = error { - let underlyingError = UnderlyingError(with: error) + let underlyingError = Self.logAndDecodeError(error) discoveredReadersSubject.send(completion: .failure(CardReaderServiceError.discovery(underlyingError: underlyingError)) ) @@ -1047,8 +1050,21 @@ private extension StripeCardReaderService { private extension StripeCardReaderService { - func internalError(_ error: Error) { - // Empty for now. Will be implemented later + static func logAndDecodeError(_ error: Error) -> UnderlyingError { + switch error { + case is UnderlyingError: + if let underlyingError = error as? UnderlyingError { + return underlyingError + } + case is CardReaderConfigError, is CardReaderServiceError: + DDLogError("💳 Card Reader Service Error: \(error)") + break + default: + let nsError = error as NSError + let errorCode = ErrorCode(_nsError: error as NSError) + DDLogError("💳 Stripe Error Code: \(errorCode.code)") + } + return UnderlyingError(with: error) } } From f1da87f29dcb60cbd7d6eaceff964bad810c567b Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 16 Sep 2024 14:03:46 +0100 Subject: [PATCH 2/5] Add Stripe error logs to 20.5 release notes --- RELEASE-NOTES.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 8d7e0de14b8..4cf4efd9584 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -4,6 +4,7 @@ 20.5 ----- - [*] Blaze: Schedule a reminder local notification asking to continue abandoned campaign creation flow. [https://github.com/woocommerce/woocommerce-ios/pull/13950] +- [internal] Mobile Payments: Log errors from Stripe Terminal [https://github.com/woocommerce/woocommerce-ios/pull/13976] 20.4 From 615a353ca776f4e9e315e8dff88d0c672f6ead6c Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 16 Sep 2024 14:18:04 +0100 Subject: [PATCH 3/5] Separately label two types of card service errors --- .../StripeCardReader/StripeCardReaderService.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Hardware/Hardware/CardReader/StripeCardReader/StripeCardReaderService.swift b/Hardware/Hardware/CardReader/StripeCardReader/StripeCardReaderService.swift index 8c5735c242b..9f63f2547f8 100644 --- a/Hardware/Hardware/CardReader/StripeCardReader/StripeCardReaderService.swift +++ b/Hardware/Hardware/CardReader/StripeCardReader/StripeCardReaderService.swift @@ -1056,9 +1056,10 @@ private extension StripeCardReaderService { if let underlyingError = error as? UnderlyingError { return underlyingError } - case is CardReaderConfigError, is CardReaderServiceError: + case is CardReaderServiceError: DDLogError("💳 Card Reader Service Error: \(error)") - break + case is CardReaderConfigError: + DDLogError("💳 Card Reader Config Error: \(error)") default: let nsError = error as NSError let errorCode = ErrorCode(_nsError: error as NSError) From 2e6aa7606f5fbc39c3c5add1c43e79fd51a58431 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 16 Sep 2024 14:19:35 +0100 Subject: [PATCH 4/5] Add note about multiple logging of errors --- .../CardReader/StripeCardReader/StripeCardReaderService.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Hardware/Hardware/CardReader/StripeCardReader/StripeCardReaderService.swift b/Hardware/Hardware/CardReader/StripeCardReader/StripeCardReaderService.swift index 9f63f2547f8..32557d5c82d 100644 --- a/Hardware/Hardware/CardReader/StripeCardReader/StripeCardReaderService.swift +++ b/Hardware/Hardware/CardReader/StripeCardReader/StripeCardReaderService.swift @@ -1053,6 +1053,8 @@ private extension StripeCardReaderService { static func logAndDecodeError(_ error: Error) -> UnderlyingError { switch error { case is UnderlyingError: + // It's possible for errors to pass through this function more than once. + // In that scenario, we don't want to log them a second time, so we can just return. if let underlyingError = error as? UnderlyingError { return underlyingError } From ade241b312806f3a7558963dfaf4aec218e24969 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Mon, 16 Sep 2024 18:03:01 +0100 Subject: [PATCH 5/5] Remove unused variable --- .../CardReader/StripeCardReader/StripeCardReaderService.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Hardware/Hardware/CardReader/StripeCardReader/StripeCardReaderService.swift b/Hardware/Hardware/CardReader/StripeCardReader/StripeCardReaderService.swift index 32557d5c82d..fc8644ae50f 100644 --- a/Hardware/Hardware/CardReader/StripeCardReader/StripeCardReaderService.swift +++ b/Hardware/Hardware/CardReader/StripeCardReader/StripeCardReaderService.swift @@ -1063,7 +1063,6 @@ private extension StripeCardReaderService { case is CardReaderConfigError: DDLogError("💳 Card Reader Config Error: \(error)") default: - let nsError = error as NSError let errorCode = ErrorCode(_nsError: error as NSError) DDLogError("💳 Stripe Error Code: \(errorCode.code)") }