search

paypal / paypal-ios

One merchant integration point for all of PayPal's services
Apache License 2.0
59 stars 27 forks source link

FPTI Updates #208

Closed jaxdesmarais closed 1 year ago

jaxdesmarais commented 1 year ago

Reason for changes

Update component name to match Braintree formatting (BT PR with this change) as will as send correlation ID where possible (BT PR with this change)

Feedback: currently the correlation ID was only added to the MXO flow. On BT PayPal Web we pull this value from the request (if present) or the Data Collector (source code). PPCP PayPal Web does not have the Data Collector as a dependency - we should consider if we want to add this dependency in a future PR or leave the correlation ID as is - only existing in NXO.

Note: We will make this same change on Android as well.

Summary of changes

Checklist

Authors

scannillo commented 1 year ago

The API requests in the CardModule also return a riskCorrelationID as a response header (see Android).

Should we also log the correlationID in the card module where possible?

It also looks like we're mis-aligned with Android since they return riskCorrelationID in merchant-facing errors and iOS is not (slightly separate concern though).

jaxdesmarais commented 1 year ago

The API requests in the CardModule also return a riskCorrelationID as a response header (see Android).

Adding breakpoints in the request/response I am not seeing those headers. Additionally looking at confirm-payment-source in PPaaS here. It looks like the correlation ID is returned as the debug ID from this endpoint in the response body. I assume that header came from somewhere on the Android side, but not seeing it when triggering an error in the raw response.

Should we also log the correlationID in the card module where possible?

The networking layers and confirm payment source request/response parsing on Android and iOS are pretty divergent right now. I put up this commit which extracts the correlation/debug ID from the httpResponse to allow us to pass it in with errors. Curious to hear your thoughts - it's not the cleanest solution but works well without fully redoing how we make these requests/responses on iOS for cards.

It also looks like we're mis-aligned with Android since they return riskCorrelationID in merchant-facing errors and iOS is not (slightly separate concern though).

I think we can probably punt on this for now. We should align card as a whole more closely across the platforms as they are pretty different right now.