w3c / payment-request

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

Clarify the behavior of calling PaymentResponse.complete() while [[retryPromise]] is pending #795

Closed aestes closed 5 years ago

aestes commented 5 years ago

I'm working on implementing payment retry in WebKit, and I'm seeing an unexpected failure when running retry-method-manual.https.html:

FAIL Calling complete() while a retry() is in progress results in an InvalidStateError. promise_test: Unhandled rejection with value: object "InvalidStateError: The object is in an invalid state."

I think I've implemented complete() as-specified, so I'm not sure whether the test is wrong or the spec is.

What's happening in the test is:

The test seems to expect that response should remain non-complete after the first call to complete() and therefore the second call to complete() should succeed since the retry promise had settled.

The spec says that response.[[complete]] should be set to true before checking if response.[[retryPromise]] is non-null. So that means the second call to complete() must throw an InvalidStateError even if response.[[retryPromise]] had been settled.

Should we defer setting response.[[complete]] to true until later in the algorithm? Or should we change the test to expect an exception on the second call to complete()?

aestes commented 5 years ago

+@marcoscaceres

aestes commented 5 years ago

Thinking about it, it seems like the spec does need changing. The specified behavior allows for a weird situation where the user can accept a payment that the merchant can never properly complete.

If we want calling complete() while a retry promise is pending to be a fatal error, then I think we need to reject the retry promise and close down the UI in addition to setting complete to true.

If we want it to be non-fatal, then I think the right fix is to throw an InvalidStateError when there is a retry promise but not set complete to true until right before the "in parallel" steps start executing.

aestes commented 5 years ago

(I'm partial to it being non-fatal.)

marcoscaceres commented 5 years ago

Thanks for all the details. Let’s fix this up during TPAC.

marcoscaceres commented 5 years ago

I went with the non-fatal option in the PR :) Well spotted, btw!