w3c / payment-request

Payment Request API
https://www.w3.org/TR/payment-request/
Other
489 stars 135 forks source link

Fine-grained error reporting with canMakePayment() #847

Open ianbjacobs opened 5 years ago

ianbjacobs commented 5 years ago

I am creating this issue based on discussion on issue #843.

I am hearing several use cases for fine-grained error reporting when canMakePayment() returns false:

I propose that we take up these use cases after version 1.

rsolomakhin commented 5 years ago
  • An error message (e.g., "okToUseAPI") that indicates that the user does not want to share detailed information but would like to use the API under show() conditions would reveal some information about the user's browser configuration.

After thinking about this more, let's not add this state for canMakePayment, because it seems to be equivalent to canMakePayment returning true in the current version of the spec: true should be returned when a payment handler is available regardless of presence of any instruments in the payment handler. If there're no payment handlers available, then show() would be rejected with NotSupportedError, which means it's not "ok to use API."

Personally, I would be OK to not add this feature to hasEnrolledInstrument either and allow browsers to always return the same value as canMakePayment regardless of user wallet contents. However, if the group consensus is to be more granular here, then we can add this state to hasEnrolledInstrument as "NotAllowedError: User has turned off sharing this information."

  • Errors in SSL. For example, self-signed SSL causes Chrome to always return "false" to canMakePayment() and reject show() with "AbortError: Request cancelled."

After version 1 is finished, let's discuss a getCannotMakePaymentReason() method .

  • Reasons why the payment sheet closed, e.g., "AbortError: User closed the payment handler", "AbortError: The payment handler cancelled the transaction", or "AbortError: User closed the card CVV entry form."

Let's recommend to be specific with error messages without requiring all browsers to use the same error message text. The error codes (e.g., AbortError) are normative, but the error messages (e.g., "Request cancelled") are informative.

I propose that we take up these use cases after version 1.

Agreed.

aestes commented 5 years ago
  • An error message (e.g., "okToUseAPI") that indicates that the user does not want to share detailed information but would like to use the API under show() conditions would reveal some information about the user's browser configuration.

After thinking about this more, let's not add this state for canMakePayment, because it seems to be equivalent to canMakePayment returning true in the current version of the spec: true should be returned when a payment handler is available regardless of presence of any instruments in the payment handler. If there're no payment handlers available, then show() would be rejected with NotSupportedError, which means it's not "ok to use API."

👍

Personally, I would be OK to not add this feature to hasEnrolledInstrument either and allow browsers to always return the same value as canMakePayment regardless of user wallet contents. However, if the group consensus is to be more granular here, then we can add this state to hasEnrolledInstrument as "NotAllowedError: User has turned off sharing this information."

👍

It's not clear to me what merchants would do with the extra granularity. hasEnrolledInstrument() returning false should not be a reason to not call show() so long as canMakePayment() returns true. Handlers can choose to offer instrument enrollment as part of their payment flow or just reject the call to show(), which as you point out offers the same information.

  • Errors in SSL. For example, self-signed SSL causes Chrome to always return "false" to canMakePayment() and reject show() with "AbortError: Request cancelled."

After version 1 is finished, let's discuss a getCannotMakePaymentReason() method .

FWIW, WebKit throws an InvalidAccessError in the PaymentRequest constructor if a non-root certificate in the chain is signed with SHA1. I don't think we reject on self-signed certificates, but maybe I'm misreading the code (and maybe we should in a way that doesn't break our testing).

We could start here (after v1) by agreeing on what exception to throw, when to throw it, and under what circumstances (and then maybe add a new method if we still think we need it).

  • Reasons why the payment sheet closed, e.g., "AbortError: User closed the payment handler", "AbortError: The payment handler cancelled the transaction", or "AbortError: User closed the card CVV entry form."

Let's recommend to be specific with error messages without requiring all browsers to use the same error message text. The error codes (e.g., AbortError) are normative, but the error messages (e.g., "Request cancelled") are informative.

👍

If we really need to be programmatically granular, I guess we can use different error codes.

I propose that we take up these use cases after version 1.

Agreed.

👍