ObjectManager interface lacks of `@throws` annotation.

[VincentLanglet]

When looking at the ObjectManager interface
https://github.com/doctrine/persistence/blob/3.2.x/src/Persistence/ObjectManager.php
there si none @throws annotation for each methods.

When looking at the ORM implementation
https://github.com/doctrine/orm/blob/2.16.x/lib/Doctrine/ORM/EntityManager.php
or the ODM implementation
https://github.com/doctrine/mongodb-odm/blob/2.6.x/lib/Doctrine/ODM/MongoDB/DocumentManager.php
a lot of exception are thrown in those methods.

The interface contract should be about the @param, @return but also @throws.
If I write

public function foo(ObjectManager $om) {
    $om->flush();
}

I have no help from PHPStorm or PHPStan that an exception can be thrown and that I should try/catch the call sometimes.

When I write

public function foo(EntityManager $om) {
    $om->flush();
}

PHPStan analysis help me to handle correctly exceptions (see https://phpstan.org/blog/bring-your-exceptions-under-control).

What could be great would be to

• have some Doctrine\Persistence\Exception class or interface
• add @throws Doctrine\Persistence\Exception to methods which can throws exception
• extends/implements this exception in the ORM/ODM repository for exception thrown in the ObjectManager methods

[malarzm]

I'm afraid we won't be able to define a good contract for exceptions... and throwing a generic DoctrineException in every other method will produce noise instead of actionable insight.

[VincentLanglet]

I'm afraid we won't be able to define a good contract for exceptions... and throwing a generic DoctrineException in every other method will produce noise instead of actionable insight.

I'm not sure to understand the issue.

Having something like

/**
 * @throws SomeExceptionInterface
 */
public function find(string $className, $id);

doesn't force you to throw generic exceptions.

You can have

class SpecificException1 implements SomeExceptionInterface {}
class SpecificException2 implements SomeExceptionInterface {}

and so on.

This could be useful to have indeed something like a DoctrineException interface and having
DoctrineORMException implements DoctrineException and DoctrineODMException implements DoctrineException.

[malarzm]

Sorry for making my thoughts so brief, let me fix that :) I tried to take a look at interface vs what exceptions ODM/ORM throw and I believe there's not much shared exceptional-state. I'm discarding all \InvalidArgumentException and its variations thrown as a poor man's typing. Also it seems that we did not annotated ODM's generic exception for situation when one is calling a method on a closed object manager, but more on this later. Without much diving into internals this leaves me with:

Interface Method	ORM	ODM	Possible common exceptions
find	OptimisticLockException ORMInvalidArgumentException TransactionRequiredException ORMException	none	Lock-related exception
persist	ORMException	none
remove	ORMException	none
clear	MappingException	none	exception is thrown because of deprecated possibility to pass class name to clear, doctrine/persistence no longer allows that
detach	none	none
refresh	ORMException TransactionRequiredException	none
flush	OptimisticLockException ORMException	none	Lock-related
getRepository getClassMetadata	none	none	MappingException could be thrown everywhere

Rest are simple methods that do not throw exceptions. Which leaves us with:

• we did not so good job in ODM with annotating exceptions
• I believe some common MappingException could be introduced here and there
• Lock-related exceptions would be good candidate but doctrine/persistence does not know about locking functionality. Both ORM and ODM are defining this on their own
• Both ORM and ODM has "closed" manager concept which again would be good candidate. But again, this is not defined in any way by doctrine/persistence currently
• Both libraries could use some kind of StorageException that would mean database-related exception happened but this would be thrown by practically every important method

[stof]

MappingException could be thrown by any method if your mapping is invalid.

And the ORM does not wrap exceptions coming from DBAL for instance (and I don't think it should as it could not wrap them in meaningful ways). @throws is useful for exceptions that you are expected to catch locally. For exceptions that indicate bugs (which require changing the code to fix them), they are best left to be handled by the global error handler, and then @throws is only creating noise.

[greg0ire]

Somewhat related: doctrine/orm#7786