Skip to content

Commit

Permalink
Stop sending error_message in PaymentSheet & Basic Integration analyt… (
Browse files Browse the repository at this point in the history
stripe#3518)

…ics, use serializeForV1Logging instead.

## Summary
<!-- Simple summary of what was changed. -->

## Motivation
https://jira.corp.stripe.com/browse/MOBILESDK-1547

## Testing
Manually tested
```
2024-04-15 17:20:52.975918-0700 BasicIntegration[14184:24531627] LOG ANALYTICS: bi_load_failed - [(key: "request_id", value: "req_ijxFj5Urxb45wb"), (key: "product", value: "STPPaymentContext"), (key: "error_type", value: "invalid_request_error"), (key: "error_code", value: "more_permissions_required"), (key: "duration", value: 4.7062259912490845)]
```
```
2024-04-15 17:21:54.558741-0700 PaymentSheetExample[14329:24533257] LOG ANALYTICS: mc_complete_payment_newpm_failure - [(key: "selected_lpm", value: "card"), (key: "request_id", value: "req_0rIJT7zevzHX0x"), (key: "pay_var", value: "paymentsheet"), (key: "ocr_type", value: "none"), (key: "locale", value: "en_US"), (key: "link_session_type", value: "shared"), (key: "link_enabled", value: true), (key: "is_decoupled", value: true), (key: "error_type", value: "card_error"), (key: "error_code", value: "card_declined"), (key: "duration", value: 9.43), (key: "deferred_intent_confirmation_type", value: "client"), (key: "currency", value: "usd"), (key: "apple_pay_enabled", value: 1), (key: "active_link_session", value: false)]
```

## Changelog
Not user facing
  • Loading branch information
yuki-stripe authored Apr 16, 2024
1 parent 84e343e commit ae6fe2e
Show file tree
Hide file tree
Showing 6 changed files with 8 additions and 60 deletions.
6 changes: 3 additions & 3 deletions Stripe/StripeiOS/Source/STPAnalyticsClient+BasicUI.swift
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@ extension STPPaymentContext {
func logLoadFailed(loadStartDate: Date, error: Error) {
let event: STPAnalyticEvent = .biLoadFailed
let duration = Date().timeIntervalSince(loadStartDate)
let params: [String: Any] = [
var params: [String: Any] = [
"duration": duration,
"error_message": error.makeSafeLoggingString(),
]
params.mergeAssertingOnOverwrites(error.serializeForV1Analytics())
log(event: event, params: params)
}

Expand Down Expand Up @@ -99,7 +99,7 @@ extension STPPaymentContext {

var params: [String: Any] = ["selected_lpm": paymentMethodType]
if let error {
params["error_message"] = error.makeSafeLoggingString()
params.mergeAssertingOnOverwrites(error.serializeForV1Analytics())
}
if let loadStartDate {
params["duration"] = Date().timeIntervalSince(loadStartDate)
Expand Down
15 changes: 0 additions & 15 deletions StripeCore/StripeCore/Source/Analytics/AnalyticLoggableError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,6 @@ extension AnalyticLoggableError where Self: Error {}

// MARK: - Error extension methods that serialize errors for analytics logging
@_spi(STP) extension Error {
/// This is like `serializeForV2Logging` but returns a single String instead of a dict.
/// TODO(MOBILESDK-1547) I don't think pattern is very good but it's here to share between PaymentSheet and STPPaymentContext. Please rethink before spreading its usage.
public func makeSafeLoggingString() -> String {
let error = self as NSError
if error.domain == STPError.stripeDomain, let code = STPErrorCode(rawValue: error.code) {
// An error from our networking layer
return code.description
} else {
// Default behavior for other errors.
// Note: For Swift Error enums, `domain` is the type name and `code` is the case index
// e.g. `LinkURLGeneratorError.noPublishableKey` -> "StripePaymentSheet.LinkURLGeneratorError, 1"
return "\(error.domain), \(error.code)"
}
}

/// Serialize an Error for logging to q.stripe.com and the `sdk.analytics_events` table
///
/// It sends the following fields:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ extension STPAnalyticsClient {
additionalParams["selected_lpm"] = paymentMethodTypeAnalyticsValue

if let error {
additionalParams["error_message"] = makeSafeLoggingString(from: error)
additionalParams.mergeAssertingOnOverwrites(error.serializeForV1Analytics())
}

for (param, param_value) in params {
Expand All @@ -328,16 +328,6 @@ extension STPAnalyticsClient {
additionalParams: additionalParams)
log(analytic: analytic, apiClient: apiClient)
}

/// Returns a string describing the provided error that doesn't contain PII and is suitable for logging.
func makeSafeLoggingString(from error: Error) -> String {
let error = error as NSError
if let error = error as? PaymentSheetError {
return error.safeLoggingString
} else {
return error.makeSafeLoggingString()
}
}
}

extension PaymentSheetViewController.Mode {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,8 @@ class PaymentSheetLoaderStubbedTest: APIStubbedTestCase {
// Should send a load failure analytic
let analyticEvent = analyticsClient._testLogHistory.last
XCTAssertEqual(analyticEvent?["event"] as? String, STPAnalyticEvent.paymentSheetLoadFailed.rawValue)
XCTAssertEqual(analyticEvent?["error_message"] as? String, "NSURLErrorDomain, -1009")
XCTAssertEqual(analyticEvent?["error_type"] as? String, "NSURLErrorDomain")
XCTAssertEqual(analyticEvent?["error_code"] as? String, "-1009")
}
wait(for: [loadExpectation], timeout: STPTestingNetworkRequestTimeout)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,8 @@ final class PaymentSheetLoaderTest: XCTestCase {
}
// Should send a load failure analytic
let analyticEvent = analyticsClient._testLogHistory.last
XCTAssertEqual(analyticEvent?["error_message"] as? String, "invalidRequestError")
XCTAssertEqual(analyticEvent?["error_type"] as? String, "invalid_request_error")
XCTAssertNotNil(analyticEvent?["error_code"] as? String)
}
}
wait(for: [loadExpectation], timeout: STPTestingNetworkRequestTimeout)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,33 +21,4 @@ class STPAnalyticsClientPaymentSheetTest: XCTestCase {
let payload = client.payload(from: analytic)
XCTAssertEqual("paymentsheet", payload["pay_var"] as? String)
}

enum MyError: Error, CustomDebugStringConvertible {
var debugDescription: String {
return "Some debug description that accidentally contains PII"
}

case foo
case bar
}

func testMakeSafeLoggingString() {
let testCases: [(Error, String)] = [
// List of inputs and expected outputs
(NSError(domain: NSURLErrorDomain, code: NSURLErrorTimedOut),
"NSURLErrorDomain, -1001"),
(PaymentSheetError.errorHandlingNextAction,
"errorHandlingNextAction"),
(NSError(domain: STPError.stripeDomain, code: STPErrorCode.cardError.rawValue),
"cardError"),
(NSError(domain: STPError.stripeDomain, code: STPErrorCode.invalidRequestError.rawValue),
"invalidRequestError"),
(MyError.bar,
"StripePaymentSheetTests.STPAnalyticsClientPaymentSheetTest.MyError, 1"),

]
for testCase in testCases {
XCTAssertEqual(STPAnalyticsClient().makeSafeLoggingString(from: testCase.0), testCase.1)
}
}
}

0 comments on commit ae6fe2e

Please sign in to comment.