Add `exceededTimeoutLimit` 3DS error #1354

stechiu · 2024-07-03T00:15:42Z

Summary of changes

Added exceededTimeoutLimit error code and error message

Checklist

Added a changelog entry

Authors

List GitHub usernames for everyone who contributed to this pull request.

@stechiu

Sources/BraintreeThreeDSecure/BTThreeDSecureV2Provider.swift

Co-authored-by: scannillo <[email protected]>

richherrera · 2024-07-11T18:16:46Z

Sources/BraintreeThreeDSecure/BTThreeDSecureV2Provider.swift

@@ -122,14 +122,20 @@ extension BTThreeDSecureV2Provider: CardinalValidationDelegate {
                completion: completionHandler
            )
        case .error, .timeout:
-            let userInfo = [NSLocalizedDescriptionKey: validateResponse.errorDescription]
+            var userInfo = [NSLocalizedDescriptionKey: validateResponse.errorDescription]


Can we remove userInfo and errorCode since we're not using them?

userInfo and errorCode get used within the case so we'll need to keep them... I can move errorCode as a global variable. Since I'm splitting off .error and .timeout, I'll keep userInfo in as local variable

jaxdesmarais · 2024-07-11T19:57:00Z

Sources/BraintreeThreeDSecure/BTThreeDSecureV2Provider.swift

+            } else if validateResponse.actionCode == .timeout {
+                errorCode = BTThreeDSecureError.exceededTimeoutLimit.errorCode
+                userInfo = [NSLocalizedDescriptionKey: BTThreeDSecureError.exceededTimeoutLimit.localizedDescription]
+                completionHandler(nil, BTThreeDSecureError.exceededTimeoutLimit)


Are we able to add a test for this?

I'll create a ticket to add tests for this and BTThreeDSecureV2Provider. There doesn't seem to be a straightforward way to test cardinal responses or action codes

jaxdesmarais · 2024-07-11T20:00:43Z

Sources/BraintreeThreeDSecure/BTThreeDSecureV2Provider.swift

+                userInfo = [NSLocalizedDescriptionKey: BTThreeDSecureError.exceededTimeoutLimit.localizedDescription]
+                completionHandler(nil, BTThreeDSecureError.exceededTimeoutLimit)
+            } else {
+                completionHandler(nil, BTThreeDSecureError.exceededTimeoutLimit)


I don't think we want our catch all else block to throw the timeout error

I think also we can on line 124 break out the case .error, .timeout: into 2 separate cases where the new timeout case can throw the new error you added.

That's a good idea, makes it cleaner

…o add-3ds-timeout-error

Sources/BraintreeThreeDSecure/BTThreeDSecureV2Provider.swift

Co-authored-by: Rich Herrera <[email protected]>

sshropshire

👏

CHANGELOG.md

jaxdesmarais · 2024-07-16T16:32:15Z

Sources/BraintreeThreeDSecure/BTThreeDSecureV2Provider.swift

+            let timeoutUserInfo = [NSLocalizedDescriptionKey: BTThreeDSecureError.exceededTimeoutLimit.localizedDescription]
+
+            if validateResponse.actionCode == .timeout {
+                errorCode = BTThreeDSecureError.exceededTimeoutLimit.errorCode
+                completionHandler(nil, NSError(domain: timeoutUserInfo.description, code: errorCode, userInfo: timeoutUserInfo))


Suggested change

let timeoutUserInfo = [NSLocalizedDescriptionKey: BTThreeDSecureError.exceededTimeoutLimit.localizedDescription]

if validateResponse.actionCode == .timeout {

errorCode = BTThreeDSecureError.exceededTimeoutLimit.errorCode

completionHandler(nil, NSError(domain: timeoutUserInfo.description, code: errorCode, userInfo: timeoutUserInfo))

completionHandler(nil, BTThreeDSecureError.exceededTimeoutLimit)

I tried running your suggestion and setting a breakpoint at validateResponse.actionCode. The errorCode returns as 0 instead of 9 when I reach the max timeout limited

9 is the error code for .exceededTimoutLimit and it's what was missing before. I know that, in theory, the suggested refactored code should work. I'm using po errorCode in the debugger but might need to test something else that returns the correct error code and error description, so let me know

🤔 if that is the case then something is certainly wrong with our logic. We do the same as my suggestion for cancel below and get the expected error code back so should be able to do the same here:

case .timeout: completionHandler(nil, BTThreeDSecureError.exceededTimoutLimit)

jaxdesmarais · 2024-07-16T16:33:15Z

Sources/BraintreeThreeDSecure/BTThreeDSecureV2Provider.swift

@@ -14,6 +14,7 @@ class BTThreeDSecureV2Provider {

    var lookupResult: BTThreeDSecureResult? = nil
    var completionHandler: (BTThreeDSecureResult?, Error?) -> Void = { _, _ in }
+    var errorCode: Int = BTThreeDSecureError.unknown.errorCode


I think we can revert this change

jaxdesmarais · 2024-07-16T16:34:49Z

Sources/BraintreeThreeDSecure/BTThreeDSecureV2Provider.swift

-            let userInfo = [NSLocalizedDescriptionKey: validateResponse.errorDescription]
-            var errorCode: Int = BTThreeDSecureError.unknown.errorCode
+        case .error:
+            let errorUserInfo = [NSLocalizedDescriptionKey: validateResponse.errorDescription]


Same here, the only diff should be on the case line. Right now we would only return if the errorNumber is 1050. We can revert the userInfo, errorCode, and completion diffs in this block

I just realized I left out this else statement that was part of the original .error case

else { completionHandler(nil, NSError(domain: BTThreeDSecureError.errorDomain, code: errorCode, userInfo: userInfo))

jaxdesmarais

Blocking for now since there are some changes that would only return an error to the merchant for 1 error number. This would cause issues for merchants getting other errors.

stechiu · 2024-07-17T00:12:01Z

Blocking for now since there are some changes that would only return an error to the merchant for 1 error number. This would cause issues for merchants getting other errors.

I've added back this missing else statement from the original code to the .error case. That will fix the issue

else {
   completionHandler(nil, NSError(domain: BTThreeDSecureError.errorDomain, code: errorCode, userInfo: userInfo))

jaxdesmarais · 2024-07-17T14:47:29Z

Sources/BraintreeThreeDSecure/BTThreeDSecureV2Provider.swift

+            let timeoutUserInfo = [NSLocalizedDescriptionKey: BTThreeDSecureError.exceededTimeoutLimit.localizedDescription]
+
+            if validateResponse.actionCode == .timeout {
+                errorCode = BTThreeDSecureError.exceededTimeoutLimit.errorCode
+                completionHandler(nil, NSError(domain: timeoutUserInfo.description, code: errorCode, userInfo: timeoutUserInfo))


🤔 if that is the case then something is certainly wrong with our logic. We do the same as my suggestion for cancel below and get the expected error code back so should be able to do the same here:

case .timeout: completionHandler(nil, BTThreeDSecureError.exceededTimoutLimit)

Sources/BraintreeThreeDSecure/BTThreeDSecureV2Provider.swift

…braintree_ios into add-3ds-timeout-error # Conflicts: # Sources/BraintreeThreeDSecure/BTThreeDSecureV2Provider.swift

…o add-3ds-timeout-error # Conflicts: # CHANGELOG.md

jaxdesmarais

👏 Thanks for addressing feedback and getting a more specific error message added for our merchants!

Add exceededTimeoutLimit 3DS error

8cccf25

stechiu requested a review from a team as a code owner July 3, 2024 00:15

Added CHANGELOG

a6a11ac

scannillo reviewed Jul 3, 2024

View reviewed changes

Sources/BraintreeThreeDSecure/BTThreeDSecureV2Provider.swift Outdated Show resolved Hide resolved

scannillo reviewed Jul 3, 2024

View reviewed changes

Sources/BraintreeThreeDSecure/BTThreeDSecureV2Provider.swift Outdated Show resolved Hide resolved

stechiu and others added 3 commits July 11, 2024 10:49

Refactored completionHandler

84bc207

Co-authored-by: scannillo <[email protected]>

Spacing

0c9576c

Co-authored-by: scannillo <[email protected]>

Merge branch 'main' into add-3ds-timeout-error

b3645d9

richherrera approved these changes Jul 11, 2024

View reviewed changes

jaxdesmarais reviewed Jul 11, 2024

View reviewed changes

stechiu and others added 3 commits July 12, 2024 16:24

Merge branch 'main' into add-3ds-timeout-error

2a45f1b

Addressed PR comments

eef1ac6

Merge branch 'main' of https://github.com/braintree/braintree_ios int…

b19d5e5

…o add-3ds-timeout-error

richherrera reviewed Jul 16, 2024

View reviewed changes

Sources/BraintreeThreeDSecure/BTThreeDSecureV2Provider.swift Outdated Show resolved Hide resolved

Addressed PR comment

7806c35

Co-authored-by: Rich Herrera <[email protected]>

sshropshire approved these changes Jul 16, 2024

View reviewed changes

jaxdesmarais reviewed Jul 16, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

jaxdesmarais reviewed Jul 16, 2024

View reviewed changes

jaxdesmarais requested changes Jul 16, 2024

View reviewed changes

Updated CHANGELOG

ed185ef

jaxdesmarais reviewed Jul 17, 2024

View reviewed changes

stechiu added 3 commits July 17, 2024 12:02

Reverted changes

764789c

Merge branch 'add-3ds-timeout-error' of https://github.com/braintree/…

c371eb1

…braintree_ios into add-3ds-timeout-error # Conflicts: # Sources/BraintreeThreeDSecure/BTThreeDSecureV2Provider.swift

Merge branch 'main' of https://github.com/braintree/braintree_ios int…

3ea47f8

…o add-3ds-timeout-error # Conflicts: # CHANGELOG.md

jaxdesmarais approved these changes Jul 17, 2024

View reviewed changes

stechiu merged commit 7c9a542 into main Jul 17, 2024
7 of 8 checks passed

stechiu deleted the add-3ds-timeout-error branch July 17, 2024 19:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `exceededTimeoutLimit` 3DS error #1354

Add `exceededTimeoutLimit` 3DS error #1354

stechiu commented Jul 3, 2024 •

edited

Loading

richherrera Jul 11, 2024

stechiu Jul 15, 2024

jaxdesmarais Jul 11, 2024

stechiu Jul 16, 2024

jaxdesmarais Jul 11, 2024

jaxdesmarais Jul 11, 2024

stechiu Jul 15, 2024

sshropshire left a comment

jaxdesmarais Jul 16, 2024

stechiu Jul 17, 2024

stechiu Jul 17, 2024

jaxdesmarais Jul 17, 2024

jaxdesmarais Jul 16, 2024

jaxdesmarais Jul 16, 2024

stechiu Jul 17, 2024

jaxdesmarais left a comment

stechiu commented Jul 17, 2024

jaxdesmarais Jul 17, 2024

jaxdesmarais left a comment

Add exceededTimeoutLimit 3DS error #1354

Add exceededTimeoutLimit 3DS error #1354

Conversation

stechiu commented Jul 3, 2024 • edited Loading

Summary of changes

Checklist

Authors

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sshropshire left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaxdesmarais left a comment

Choose a reason for hiding this comment

stechiu commented Jul 17, 2024

Choose a reason for hiding this comment

jaxdesmarais left a comment

Choose a reason for hiding this comment

Add `exceededTimeoutLimit` 3DS error #1354

Add `exceededTimeoutLimit` 3DS error #1354

stechiu commented Jul 3, 2024 •

edited

Loading