- Issue created by @GuillaumePacilly
- Merge request !10add SaferpayException class and handle payment cancellation → (Open) created by GuillaumePacilly
- 🇨🇭Switzerland berdir Switzerland
I see. Result is basically the same, but it avoids logging it as an error and displayed message is better.
seems awkward to redirect from return to cancel, only for that to then redirect to the actual desired target. Especially since that will actually all back into onCancel() of the plugin.
- 🇨🇭Switzerland berdir Switzerland
I see, a custom exception makes sense so that we can handle this differently for thee return vs notify scenario. I was thinking we could just always thow a redirect exception directly, but that's weird on the notify route.
> Also, maybe we should not always throw the base PaymentGatewayException
Nobody is going to look at it other than commerce, and all it cares about is that it's either a PaymentGatewayExeption (user error, telling the user to retry) or another exception (which tells the user that it's a system error and that they should contact support instead of retrying). IMHO, more specific exception classes are in practice not really relevant.
What is more relevant is whether or not the thrown Exception is PaymentGatewayException or a subclass of that. Should SaferpayException extend from that or not? And should we wrap other client exceptions in PaymentGatewayException. I'm tempted to say yes, because that's the current behavior, and by not doing that, it results in a different message to users. Which might be more correct, but it's maybe also unexpected, untranslated and so on.
Thoughts?