Allow payment handlers to report back errors via Payment Request API#1050
Allow payment handlers to report back errors via Payment Request API#1050marcoscaceres merged 5 commits intogh-pagesfrom
Conversation
ccb85c9 to
aaa261e
Compare
stephenmcgruer
left a comment
There was a problem hiding this comment.
Ok I've incorporated your comments (thanks!) and put together three variants:
- A version where we take the unspecified error from the platform handler, and use the "let |error| be an appropriate {{DOMException}}" wording -e9eec11
- A version where the payment handler is presumed to pass us a DOMException directly (i.e., any conversion to DOMException happens 'inside' the relevant payment handler and any spec it has) - ae1280b
- A version which uses a general JavaScript value like AbortSignal does, and rejects with that reason - d976730
I personally prefer (2), but I'm open to any of them.
d976730 to
236c39b
Compare
|
Alternative, building on @stephenmcgruer proposal: PayPal's SDK exposes Proposed SolutionUse the existing WebIDL Two
|
|
(Removing Rouslan for review as he is no longer working on Payment Request :)) |
|
I don't think this is testable... but we should file browser bugs and I need to check if WebKit is currently bailing out anywhere with |
|
We should get an Ok from the folks in #1040 before merging, in case we've missed something... but otherwise, this seems ok to me. |
|
Filed bugs for Chromium and WebKit, filed an issue for Web-Based Payment Handler API, and added the following for WPT impact:
|
Looks like we have that ok - @ianbjacobs any concerns/thoughts from your side, or are we good to land this? |
ianbjacobs
left a comment
There was a problem hiding this comment.
I defer to the implementers here; and you seem happy so I'm happy.
This is the first CL in a series of changes to allow web-based payment handlers to indicate internal error. Before these changes, if a web-based payment handler rejected the promise it passed to respondWith, it was always treated as a "user cancelled" signal. However the Payment Request and Web-based Payment Handler specifications are changing (see w3c/payment-request#1050 and w3c/web-based-payment-handler#428) to allow reporting internal error as well. This first CL makes the necessary changes on the service worker side and plumbs them through to the service worker payment app code. If the promise passed to PaymentRequestEvent.respondWith is rejected with an "OperationError" DOMException, it is now mapped to PAYMENT_EVENT_INTERNAL_ERROR instead of PAYMENT_EVENT_REJECT. This will later be used in the payments code to reject the show() promise accordingly. Bug: 473478138 Change-Id: Ia6986035865ee4f710d0a50410bcdb14a474c945 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7639049 Commit-Queue: Stephen McGruer <smcgruer@chromium.org> Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@chromium.org> Reviewed-by: Darwin Yang <darwinyang@chromium.org> Cr-Commit-Position: refs/heads/main@{#1601399}
This is the second CL in a series of changes to allow web-based payment handlers to indicate internal error. Before these changes, if a web-based payment handler rejected the promise it passed to respondWith, it was always treated as a "user cancelled" signal. However the Payment Request and Web-based Payment Handler specifications are changing (see w3c/payment-request#1050 and w3c/web-based-payment-handler#428) to allow reporting internal error as well. This CL implements support for OperationError (behind a base::Feature, kPaymentRequestSupportReportingAppError) for web-based payment handlers on Desktop only. To do so, it plumbs the mojom::PaymentEventResponseType through OnInstrumentDetailsError down to the content::PaymentRequest, which then uses it when rejecting the show() promise. A future CL will implement support for consuming the mojom::PaymentEventResponseType in the Android Payment Request implementation as well. Note that due to crbug.com/40116807, an error dialog is still shown to the user on desktop before show() is rejected. Bug: 473478138 Change-Id: Iac82bce818e47317e57bf443a8c185bab43079c0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7641581 Reviewed-by: Joe Mason <joenotcharles@google.com> Reviewed-by: Darwin Yang <darwinyang@chromium.org> Commit-Queue: Stephen McGruer <smcgruer@chromium.org> Cr-Commit-Position: refs/heads/main@{#1604437}
This is the third CL in a series of changes to allow web-based payment handlers to indicate internal error. Before these changes, if a web-based payment handler rejected the promise it passed to respondWith, it was always treated as a "user cancelled" signal. However the Payment Request and Web-based Payment Handler specifications are changing (see w3c/payment-request#1050 and w3c/web-based-payment-handler#428) to allow reporting internal error as well. This CL passes the error reason over to Chromium Android code and handles it in the Payment Request implementation there, so that web-based payment handlers on Chromium for Android are also able to indicate internal app error. As part of the change, split up the payment_app.mojom file, as the PaymentEventResponseType enum has to be made available to webview but we don't want to pull in the full dependency chain of payment_app.mojom. The split technically pulls out more than is required, but it seemed a better conceptual fit than only pulling out the specific enum. Bug: 473478138 Change-Id: If37ef9372555514493e523b8c72fda728768c357 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7693354 Reviewed-by: Dave Tapuska <dtapuska@chromium.org> Commit-Queue: Stephen McGruer <smcgruer@chromium.org> Reviewed-by: Slobodan Pejic <slobodan@chromium.org> Reviewed-by: Dominic Farolino <dom@chromium.org> Cr-Commit-Position: refs/heads/main@{#1614588}
See #1040
Co-authored by: @marcoscaceres
The following tasks have been completed:
Implementation commitment:
Optional, impact on Payment Handler spec?
w3c/web-based-payment-handler#428
Preview | Diff