Closed alexkart closed 5 years ago
Hi @alexkart
Thank you for the pull request but unfortunately, it's BC incompatible.
You see - the behavior we have right now was crafted the way it is intentionally so if we'll start throwing Exceptions it would most likely break tons of code that don't expect Exceptions right now.
To make Repository flexible and allow users to choose the behavior they want I suggest adding "errorHandlerStrategy" property with possible values as "handle", "throw error" (0 and 1). By default, it should still handle errors and if other behavior will be required - users will be able to change it.
I'll think about a better way but it would be BC incompatible so "the right solution" will be for the major release.
With best regards, Dmitry Kolodko
OK, I see. I've added the property for switching behavior and also added $errors
property to store messages from exceptions so that you can get the error message from repository instance after saving attempt.
But currently, behaviors (with transactions and without transactions) are not consistent. Without transactions repository always throws exceptions but without transactions it doesn't (by default).
@alexkart
But currently, behaviors (with transactions and without transactions) are not consistent. Without transactions repository always throws exceptions but without transactions it doesn't (by default). yes, it is indeed an issue that needs to be fixed until the next major release.
as for right now, the solution in MR is ok, just have one slight comment - please fix getters/setters formatting accordingly to a common style (open bracket position)
then it can be merged
Fixed
When using transactions repository suppresses exceptions but it should throw it back to the calling code