Payment-request: Retrying a payment

[marcoscaceres]

@domenic, @aestes, @zkoch, @adrianba, @mnoorenberghe, @stpeter, @edenchuang, all, here is an attempt to solve the retry issue. It builds on the fine grained error recovery proposal at #647, and on payment method change event (#695).

Retrying a payment flow

I did a bunch of experimentation, and I think the most logical solution is to mixin the handlers from PaymentRequest into PaymentResponse (or just add them directly as below?... hi @domenic!), plus add a onpayerdetailschange and onpaymentmethodchange event handlers, and a .retry() method.

That means that we don't need to screw around with dead PaymentRequests by resetting their state machines.

The rationale for adding the new onpayerdetailschange being that PaymentRequest never gets things like payerName, payerEmail, etc. so there is no way to detect those changing. Similarly, onpaymentmethodchange reflects the full payment handler response, which is not available on PaymentRequest.

Thus... I'd like to propose:

IDL changes

/* see https://github.com/w3c/payment-request/issues/647#issuecomment-385852164 */
dictionary PaymentErrors {
  PayerErrors payerErrors;
  AddressErrors shippingAddressErrors;
}

partial interface PaymentResponse {
  attribute EventHandler onpayerdetailschange;
  attribute EventHandler onpaymentdetailschange;
  Promise <void> retry(PaymentErrors errors);
}

retry() method

The retry() method signals that something is wrong with the sheet.
What's actually wrong with the sheet is represented by the PaymentErrors errors argument.

It returns a promise that resolves when the user hits Pay again (after they hopefully fix errors), and gets rejected if the user aborts (e.g., they hit the "esc" key while the sheet is showing).

retry() can only be called multiple times, but only after each returned promise settles. It tells the browser: "let the user change the requested inputs" (i.e., what's in PaymentOptions and the ). Calling it prematurely, rejects a retryPromise with "InvalidStateError" .

Empty PaymentErrors dictionary (i.e., .retry({})) means "unknown" error, meaning the user should check all their inputs. Otherwise, fix the errors.

onpayerdetailschange event handler

The onpayerdetailschange fires when the user changes name, email, phone, depending on whether PaymentOptions requested them.

When this event fires, the merchant simply queries PaymentResponse for the thing they are interested in validating/checking. If value of attribute is invalid, the merchant calls ev.updateWith({ stuffToFix }) (see #647).

Example of usage

This shows how a merchant could use async validators to process a PaymentResponse.
(Mock code, untested... treat a pseudo code for illustrative purposes).

async function doPaymentRequest() {
  const request = new PaymentRequest(methodData, details, options);
  const response = await request.show();
  const validator = new Validator(); // user code
  // collect any errors from response
  const {
    shippingAddressErrors,
    payerErrors,
    paymentMethodErrors, // <- needs exploration.
  } = await validator.validateResponse(response);
  // Ok, we got bad input... let's get the user to fix those!
  if (shippingAddressErrors || payerErrors || paymentMethodErrors) {
    const promisesToFixThings = [];
    // let's make sure the shipping address is fixed
    if (shippingAddressErrors) {
      const promiseToFixAddress = new Promise(resolve => {
        // Browser keeps calling this until promise resolves.
        response.onpaymentaddresschange = async ev => {
          const promiseToValidate = validator.validateShippingAddress(response);
          ev.updateWith(promiseToValidate);
          // we could abort here via try/catch
          const errors = await promiseToValidate;
          if (!errors) {
            resolve(); // yay! Address is fixed!
          }
        };
      });
      promisesToFixThings.push(promiseToFixAddress);
    }
    if (payerErrors) {
      // As above for payer errors
    }
    response.retry({ shippingAddressErrors, payerErrors });
    await Promise.all(promisesToFixThings);
  }
  await response.complete("success");
}
doPaymentRequest();

[domenic]

I think I'm missing something more fundamental in the explanation here. Why is this method needed at all? Let's say that the page signals an error using updateWith(paymentDetailsUpdate). Can't the user then just fix the error and press "pay" again? What does this retry() method add?

[marcoscaceres]

Today, the merchant receives response after the user clicks “Pay” - so access to .updateWith(), which is only available via PaymentRequestUpdateEvent, is no longer accessible to them. So now they only have one option: call response.complete(validationResult). Remember also that at this point, the payment sheet is showing only a spinner, waiting for the payment to complete().

So, there is fundamentally no way to for the merchant to signal “something is wrong with their email, but the user can fix it!”. That’s what .retry() does: it removes the spinner, and brings back the form where the user can fix, for instance, their email.

Hopefully that makes sense.

[domenic]

Got it, thanks! I knew I was missing something obvious.

It seems like it would be kind of nice if this were tied to complete(); e.g. complete("fail-but-fixable", errors). One idea is that the promise returned by such a complete("fail-but-fixable") call stays pending until the user fixes their mistake and presses "Pay" again, at which point the other PaymentResponse attributes are hopefully changed, and they can validate and call response.complete("success"). Maybe that's too weird though...

Basically my confusion is that it seems like we're telling people to sometimes call complete() when they get a response, and sometimes call retry() when they get a response. This muddles the previously clear path of "always call complete()".

[marcoscaceres]

This muddles the previously clear path of "always call complete()".

Agree, but kinda feel it’s ok here. The code paths all lead to a single .complete() call, even if the merchant neends to retry(). The flow is still nice:

1. Get response.
2. Validate response.
3. Optionally, retry():
   
   
   
   1. Await things to get fixed user.
   
   
4. complete().

[domenic]

I guess that makes sense.

I wonder if there's a way to make "await things getting fixed by the user easier". Maybe if retry() returned a promise that resolved next time the "Pay" button was pressed?

In particular, I'm concerned about the complexity of your sample code for a merchant that just wants to do validation all at once, without dealing with the *change events. They just want to wait for the pay button to be clicked, check the fields, and ask for a retry if the fields are wrong. Then repeat until the fields are right.

[marcoscaceres]

Maybe if retry() returned a promise that resolved next time the "Pay" button was pressed?

Ah, great point.

In particular, I'm concerned about the complexity of your sample code for a merchant that just wants to do validation all at once, without dealing with the *change events. They just want to wait for the pay button to be clicked, check the fields, and ask for a retry if the fields are wrong. Then repeat until the fields are right.

Right, so:

// Recursive validation, no events.
async function validateResponse(r){
  const errors = validate(r);
  if (errors) {
     await r.retry(errors);
     await validateResponse(r);
  }
}

We could, in theory, support both models: The nice thing about the events is that it allows more immidiate feedback between the user and merchant... like: user changes their email, merchant checks it right after it changes. Merchant calls .updateWith() saying it's still bad... repeat until it's good.

Thus, the problem with just retry() is that the user is put into a situation where "Pay" means "validate"... I guess it already kinda means that, but without the events, that becomes a bit more extreme.

[domenic]

Yeah, supporting both models makes sense to me.

As an aside: is there no way today to say why a complete("fail") failed?

[marcoscaceres]

As an aside: is there no way today to say why a complete("fail") failed?

Nope. The API expects the merchant to explain why it failed after the sheet disappears.

I updated the .retry() proposal above. Now returns the promise, can be called multiple times.

[marcoscaceres]

Ok, some more fixes above. Oooh! this is really nice now. Thanks @domenic!

I'll try to come up with a cleaner example for handling the eventing path. The above makes it look worst than it is, I think.

[marcoscaceres]

Ok, .retry() with recursive validation with event support (my submission for JS1k this year?):

async function doPaymentRequest() {
  const request = new PaymentRequest(methodData, details, options);
  const response = await request.show();
  await recursiveValidate(response);
  await response.complete("success");
}

async function recursiveValidate(response) {
  const promisesToFixThings = [];
  const errors = await validate(response);
  if (!errors) {
    return;
  }
  if (errors.shippingAddressErrors) {
    const promise = fixField(response, "shippingaddresschange", shippingValidator);
    promisesToFixThings.push(promise);
  }
  if (errors.payerErrors) {
    const promise = fixField(response, "payerdetailschange", payerValidator);
    promisesToFixThings.push(promise);
  }
  await Promise.all([response.retry(errors), ...promisesToFixThings]);
  await recursiveValidate(response);
}

function fixField(response, event, validator) {
  return new Promise(resolve => {
    // Browser keeps calling this until promise resolves.
    response.addEventListener(event, async function listener(ev) {
      const promiseToValidate = validator(response);
      ev.updateWith(promiseToValidate);
      const errors = await promiseToValidate;
      if (!errors) { // yay! fixed!
        event.removeEventListener(event, listener);
        resolve();
      }
    });
  });
}

doPaymentRequest();

[marcoscaceres]

I've been investigating how to do add .retry() over the last few days... Because of the way we originally wrote the spec, we never assumed PaymentRequest and PaymentResponse would take the same code paths (or that the PaymentResponse would need a state machine)... thus, the spec is heavily biased towards PaymentRequest in all its algorithms, and to change it means a significant rewrite.

Because of this, I'm going to investigate restarting the PaymentRequest state machine. Thus, going to see what happens if .retry() puts PaymentRequest into the "interactive" state... and then we can figure out where to fire payerdetailschange.

[ianbjacobs]

See related issue to update the diagram:
https://github.com/w3c/payment-request/issues/728

[marcoscaceres]

As retry() itself has landed in the spec, I’m inclined to close this. There are a few additional things landing related to retry(), but they have separate issues filed.

[Dew0Tgx]

async function doPaymentRequest() {
const request = new PaymentRequest(methodData, details, options);
const response = await request.show();
const validator = new Validator(); // user code
// collect any errors from response
const {
shippingAddressErrors,
payerErrors,
paymentMethodErrors, // <- needs exploration.
} = await validator.validateResponse(response);
// Ok, we got bad input... let's get the user to fix those!
if (shippingAddressErrors || payerErrors || paymentMethodErrors) {
const promisesToFixThings = [];
// let's make sure the shipping address is fixed
if (shippingAddressErrors) {
const promiseToFixAddress = new Promise(resolve => {
// Browser keeps calling this until promise resolves.
response.onpaymentaddresschange = async ev => {
const promiseToValidate = validator.validateShippingAddress(response);
ev.updateWith(promiseToValidate);
// we could abort here via try/catch
const errors = await promiseToValidate;
if (!errors) {
resolve(); // yay! Address is fixed!
}
};
});
promisesToFixThings.push(promiseToFixAddress);
}
if (payerErrors) {
// As above for payer errors
}
response.retry({ shippingAddressErrors, payerErrors });
await Promise.all(promisesToFixThings);
}
await response.complete("success");
}

• [x] - doPaymentRequest();
  async function doPaymentRequest() {
  const request = new PaymentRequest(methodData, details, options);
  const response = await request.show();
  const validator = new Validator(); // user code
  // collect any errors from response
  const {
  shippingAddressErrors,
  payerErrors,
  paymentMethodErrors, // <- needs exploration.
  } = await validator.validateResponse(response);
  // Ok, we got bad input... let's get the user to fix those!
  if (shippingAddressErrors || payerErrors || paymentMethodErrors) {
  const promisesToFixThings = [];
  // let's make sure the shipping address is fixed
  if (shippingAddressErrors) {
  const promiseToFixAddress = new Promise(resolve => {
  // Browser keeps calling this until promise resolves.
  response.onpaymentaddresschange = async ev => {
  const promiseToValidate = validator.validateShippingAddress(response);
  ev.updateWith(promiseToValidate);
  // we could abort here via try/catch
  const errors = await promiseToValidate;
  if (!errors) {
  resolve(); // yay! Address is fixed!
  }
  };
  });
  promisesToFixThings.push(promiseToFixAddress);
  }
  if (payerErrors) {
  // As above for payer errors
  }
  response.retry({ shippingAddressErrors, payerErrors });
  await Promise.all(promisesToFixThings);
  }
  await response.complete("success");
  }
  doPaymentRequest();