consider requiring an error (non-empty dict) for .retry()?

[marcoscaceres]

Right now, .retry() can accept an empty dictionary, in which case the expectation is that the browser displays a custom error message telling the user to try the payment again.

Should we be more strict about that by requiring that the dictionary have at least one valid error-related member?

@romandev, @rsolomakhin, what does Chrome do with .retry({/* no dictionary members*/})?

@aestes, same question about Safari?

[romandev]

During code review for retry() in Chromium side, there was a similar discussion. https://chromium-review.googlesource.com/c/chromium/src/+/1144587/8/components/payments/content/payment_request.cc#206

I think we should allow retry({});. If there is no valid error-related member, UA can display default value.

[marcoscaceres]

I think we should allow retry({});. If there is no valid error-related member, UA can display default value.

I tend to feel the same, but that's my personal position.

@mnoorenberghe from our front end team feels there should be at least one valid (#793) member present.

Would appreciate additional input from other implementers - @zkoch opinions?

[mnoorenberghe]

The UA would only be guessing about what the issue was so wouldn't be able to communicate anything meaningful to the user. The merchant has context to provide the user so that's what they should do. I can't think of a valid reason for a merchant to not provide their own error… it's more likely a bug in their code and so an exception would be more useful IMO.

[marcoscaceres]

I'd like to be careful we don't go too deep here, because a merchant could also screw up and do:

.retry({error: ""}); // just as bad as "{}"? 

Which is why I'd still argue that, {} === browser saying, "There was an issue processing the error, try again?"

[rsolomakhin]

I prefer to require at least one non-empty error.

[marcoscaceres]

@rsolomakhin, for empty display item label, Chrome warns if it's empty but doesn't throw. Maybe we should do the same here?

So, we can require at least one "valid" one. But console.warn if empty... I don't want to get into the game of checking, for instance:

.retry({error: " "}); //single space
.retry({error: "     "});
.retry({error: "\n"});
.retry({error: "\t"});

Alternatively, for all error label types, we trim and check if not empty string. Throw if empty.

[rsolomakhin]

Alternatively, for all error label types, we trim and check if not empty string. Throw if empty.

I like the trimming approach and would be OK warning instead of throwing, if you prefer.

[marcoscaceres]

Ok, let's require 1 valid, then trim, and, if empty, throw... it will be useful and "exceptionally fun" 😂👍 (this whole thread was just an excuse to make that joke). I'll send a PR.

[aestes]

In Safari it's ok to have an empty string as the error message, at least for payer errors and address errors. The field itself conveys useful information, and Safari will display a default message. So .retry({ payer: { phone: "" }}) would at least highlight the phone number field in red and display a default error like "Phone Number Invalid."

[marcoscaceres]

Fwiw, the way @aestes describes it is how I envisioned it should work too.

Will discuss internally with our front-end folks and will contemplate a bit more. Keeping compat with Safari seems like a good thing.

[rsolomakhin]

Keeping compat with Safari seems like a good thing.

Agreed.

[marcoscaceres]

@mnoorenberghe, does this work for you? We go with the compromise solution of following Safari wrt "", but we validate that members make sense.

[mnoorenberghe]

I don't think that's a great API design as I can see authors doing something like:

payer: {
phone: isValid(phone) ? "Phone is invalid" : ""; 
}

It's also inconsistent with setCustomValidity which is a similar API. There "" means "clear the error" which is the opposite of what's being proposed.

This will add more complexity for Firefox because internally we are using setCustomValidity.

I guess since we're currently using DOMString, authors can't use "", null, or undefined in a ternary to mean "no error" like in my example above as it won't give the expected result. I wonder if we should have used DOMString? to allow that?

What's the rational for Safari having this behaviour? So that merchants don't have to provide their own strings and therefore have inconsistency between sites?

If there is a good reason to provide a default message on empty, or there's not and Apple doesn't want to change this, I guess that's ok with me but only if this behaviour is specified in the spec (and documented on MDN).

[marcoscaceres]

I don't think that's a great API design as I can see authors doing something like:

Agree, but it's not uncommon with dictionaries to do:

{
  phone: isValid(phone) ? "Phone is invalid" : undefined; 
}

Which the IDL layer will treat as "not passed".

I guess since we're currently using DOMString, authors can't use "", null, or undefined in a ternary to mean "no error" like in my example above as it won't give the expected result. I wonder if we should have used DOMString? to allow that?

Making null mean default also feels a bit weird, tbh.

What's the rational for Safari having this behaviour? So that merchants don't have to provide their own strings and therefore have inconsistency between sites?

@aestes? (I'm guessing is for consistently error messages that also get localized for free)

I'm happy to concur on this. Worst case, internally, in Firefox:

payerInput.setCustomValidity( 
  "payer" in errors === false ?  "" : errors.payer === "" ? defaultError : errors.payer
) 

[aestes]

@aestes? (I'm guessing is for consistently error messages that also get localized for free)

In ApplePayError the message is optional, and we have localized fallback messages for each combination of errorCode and optional contactField.

The rationale is that most merchants just want to communicate which fields are invalid so that Safari will highlight them; they don't necessarily want to communicate a custom error message and worry about localizing it.

[mnoorenberghe]

How will other browsers handle the generic error property being empty?

{
  error: ""
}

It seems like it'd be quite hard to provide a generic message in this case as it could apply to the shipping address or payment method.

[marcoscaceres]

@mnoorenberghe maybe we should leave this behaviour (error: "") to implementations, and show a console warning?

[mnoorenberghe]

Maybe… we should at least fix retry() and retry({}) if possible though maybe the latter gets handled as error: ""?

[marcoscaceres]

Sent PR based on discussion above: https://github.com/w3c/payment-request/pull/825 - should match Safari's behavior now, but need to test it.