Error events for 3ds2/redirect/encryption #1918
Conversation
erenbesel
added
the
chore
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
✅ No changes detectedComparing Analyzed targets: Adyen, AdyenActions, AdyenCard, AdyenCashAppPay, AdyenComponents, AdyenDelegatedAuthentication, AdyenEncryption, AdyenSession, AdyenSwiftUI, AdyenTwint, AdyenWeChatPay |
|
| @@ -58,9 +58,19 @@ extension CardComponent { | |||
|
|
|||
| submit(data: data) | |||
| } catch { | |||
| sendEncryptionErrorEvent() | |||
There was a problem hiding this comment.
are we not interested in the actual error message?
| @@ -462,9 +462,19 @@ extension GiftCardComponent { | |||
| storePaymentMethod: false | |||
| )) | |||
| } catch { | |||
| sendEncryptionErrorEvent() | |||
There was a problem hiding this comment.
Same here - is the error description irrelevant?
| internal var checkoutAttemptId: String? { | ||
| didSet { | ||
| if checkoutAttemptId != nil { | ||
| startNextTimer() |
There was a problem hiding this comment.
This is a bit bigger than the scope of this. Any reason to change this?
I quickly looked through tests changes and didn't find any new/changed tests for this part of logic.
| subType: subtype | ||
| ) | ||
| context.analyticsProvider?.add(log: logEvent) | ||
| private func sendErrorEvent(_ code: AnalyticsConstants.ErrorCode, for component: String, message: String? = nil) { |
There was a problem hiding this comment.
There is some inconsistency between method name and it's parameters.
First one with name omitted is code and from the outside it suggests and event (sendErrorEvent).
I would suggest to make it more consistent to not accumulate ambiguity.
It also does two things at a time:
- creates an instance of
AnalyticsEventErrorwhich is not a direct responsibility ofThreeDS2CoreActionHandler - asks to send it
I see we now have AnalyticsEventError instantiation across multiple files, do you think we can extract it so in the end we only send events using a predefined enum(s) from specific handlers such as ThreeDS2CoreActionHandler?
| type: .internal | ||
| ) | ||
| errorEvent.code = AnalyticsConstants.ErrorCode.encryptionError.stringValue | ||
| context.analyticsProvider?.add(error: errorEvent) |
There was a problem hiding this comment.
Double dot syntax is always a trigger. It is something that would make refactor in the future really hard (extracting analytics out of context for example). We can conform context to analytics handling protocol (we can create one if necessary) and just proxy all add() calls for now.
| /// Struct to hold error codes as type-safe static variables. | ||
| public struct ErrorCode { | ||
|
|
||
| public static let redirectFailed = ErrorCode(600) |
There was a problem hiding this comment.
There is a bunch of code.stringValue across this PR, is there a reason to make it Int instead of having an enum or just using a String raw value?
Summary
Several error events for:
Ticket
COIOS-841