thephpleague / omnipay-common

Core components for the Omnipay PHP payment processing library
MIT License
329 stars 242 forks source link

Added not found exception #187

Open ptuchik opened 6 years ago

ptuchik commented 6 years ago

Thrown when the requested resource is not found on payment gateway to differentiate from other invalid responses

barryvdh commented 6 years ago

What do you think about the name. SHould it be more descriptive? ResourceNotFoundException perhaps?

@judgej thoughts?

ptuchik commented 6 years ago

Can be, it is just a bit longer. Let me know please if I have to change or not. My project is waiting for these updates :)

judgej commented 6 years ago

Are any of the other exceptions thrown as a result of a specific gateway request error, or does this one stand alone (possibly one of a group of related exceptions)? Just wondering (if I understand correctly) whether exceptions from the remote gateway complaining should be namespaced together, with a generic exception and more detailed exceptions extended by the drivers?

ptuchik commented 6 years ago

The original integration was throwing only SDK’s exceptions, currently I have PR overriding only “customer not found” error, to be able to catch and recreate profile instead of showing error to user. In future we can try to morph all errors to Omnipay’s exceptions

barryvdh commented 6 years ago

It can throw an InvalidResponse but that's not really specific.

judgej commented 6 years ago

What's throwing it - omnipay-common or the driver?

Assuming it's the driver, the driver could extend InvalidResponse with something that is appropriate to that driver. A few common extensions (stuff REST is generally involved in) could be added to omnipay-common to start with, but it would allow a driver to add its own, e.g. NoPermissionResponse but still be caught as a generic "problem talking to the gateway" exception by the merchant site if it wants to.

Anyway - just a thought - I'm no expert at structuring exceptions, and a little behind at what PHP 7.2+ is bringing to the exception table.

ptuchik commented 6 years ago

@judgej Totally agreed with you, I just suggested to throw this generic Omnipay's exception, to not hardcode Gateway-specific namespaces in multi-gateway application to be able to catch them all

ptuchik commented 6 years ago

Changed to ResourceNotFoundException