exception refactoring to only use doctrine exceptions #322

merged 1 commit into from Sep 13, 2013


None yet

3 participants

dbu commented Sep 2, 2013
Bug fix? yes
New feature? yes
BC breaks? yes
Deprecations? no
Tests pass? yes
Fixed tickets PHPCR-71
License MIT
Doc PR --

This is a minimal PR to not throw any \InvalidArgumentExceptions or \RuntimeException anymore but all based on PHPCRException or doctrine commons MappingException. What could still be done is moving exception messages into factory methods to clean up the code, but that will be no BC break (created http://www.doctrine-project.org/jira/browse/PHPCR-116)

@dbu dbu commented on the diff Sep 2, 2013
@@ -28,7 +30,7 @@
* @author Benjamin Eberlei <kontakt@beberlei.de>
* @author Lukas Kahwe Smith <smith@pooteeweet.org>
-class MappingException extends \Exception
+class MappingException extends BaseMappingException
dbu Sep 2, 2013 Member

not 100% sure if this is what we should do - we can't extend PHPCRException at the same time. then again, mapping problems are in develepment or on the console so exception catching and handling is probably irrelevant.

@lsmith77 lsmith77 merged commit 49dca3c into master Sep 13, 2013

1 check passed

default The Travis CI build passed
@lsmith77 lsmith77 deleted the exception-cleanup branch Sep 13, 2013
@lsmith77 lsmith77 referenced this pull request Sep 13, 2013

[WIP] Query Builder V2 #318

8 of 8 tasks complete

Sorry to get to the party rather later, but would it not make sense to have the exceptions in Doctrine\ODM\PHPCR\Exception\{InvalidArgument,Runtime,}Exception ?

Seems this is what doctrine does: use Doctrine\Common\Proxy\Exception\InvalidArgumentException


this was just a quick fix but not really a proper solution. ideally we should indeed use those exceptions along with maybe an interface to make it possible to catch all PHPCR ODM exception in one swoop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment