Store exception thrown during UnitOfWork::commit in the entity manager to allow easier debugging #6953

New Issue

admin · 2026-01-22T15:42:05+01:00

admin commented

2026-01-22 15:42:05 +01:00

Originally created by @pkly on GitHub (Mar 14, 2022).

Feature Request

Currently if you have a lot of code that uses the entity manager you will have a lot of try/catch blocks, assuming you actually catch all the exceptions you might have.
Exceptions may occur during doctrine events, be related to doctrine itself, your connection and so on. The list is rather long.

What I'd like to propose is a change in the interface and default EntityManager implementation, which should be backwards compatible and be an overall QoL feature.

Q	A
New Feature	yes
RFC	no
BC Break	no

Summary

Looking at the UnitOfWork code in the commit method you can see it catches any exceptions and re-throws them, which is fine. But if not caught in that block it becomes essentially impossible to actually find out what happened.

I propose the following

// EntityManagerInterface.php

public function close(?\Throwable $e = null);

// EntityManager.php

private ?\Throwable $lastException = null;

// [...]

public function close(?\Throwable $e = null)
{
    $this->clear();
    $this->lastException = $e;

    $this->closed = true;
}

// [...]

private function errorIfClosed(): void
{
    if ($this->closed) {
        throw ORMException::entityManagerClosed($this->lastException);
    }
}

In ORMException we'd just need to add 0, $e as parameters to self and it should just work.

This change would allow easier debugging in more complex applications with no input from the programmer while using comfortable exception stacks.
If accepted I could submit a PR for this feature.

Originally created by @pkly on GitHub (Mar 14, 2022). ### Feature Request Currently if you have a lot of code that uses the entity manager you will have a lot of try/catch blocks, assuming you actually catch all the exceptions you might have. Exceptions may occur during doctrine events, be related to doctrine itself, your connection and so on. The list is rather long. What I'd like to propose is a change in the interface and default EntityManager implementation, which should be backwards compatible and be an overall QoL feature. | Q | A |------------ | ------ | New Feature | yes | RFC | no | BC Break | no #### Summary Looking at the `UnitOfWork` code in the commit method you can see it catches any exceptions and re-throws them, which is fine. But if not caught in that block it becomes essentially impossible to actually find out what happened. I propose the following ```php // EntityManagerInterface.php public function close(?\Throwable $e = null); ``` ```php // EntityManager.php private ?\Throwable $lastException = null; // [...] public function close(?\Throwable $e = null) { $this->clear(); $this->lastException = $e; $this->closed = true; } // [...] private function errorIfClosed(): void { if ($this->closed) { throw ORMException::entityManagerClosed($this->lastException); } } ``` In ORMException we'd just need to add 0, $e as parameters to self and it should just work. This change would allow easier debugging in more complex applications with no input from the programmer while using comfortable exception stacks. If accepted I could submit a PR for this feature.

admin closed this issue

2026-01-22 15:42:05 +01:00

admin commented

2026-01-22 15:42:06 +01:00

@greg0ire commented on GitHub (Mar 14, 2022):

BC Break | no

Changing an interface is a BC-break, so is changing the signature of a non-final method of a non-final class.

But if not caught in that block it becomes essentially impossible to actually find out what happened.

Please elaborate on this. If the exception is not caught, you get a stack trace, so it's not completely clear to me what you are complaining about here.

@greg0ire commented on GitHub (Mar 14, 2022): > BC Break | no Changing an interface is a BC-break, so is changing the signature of a non-final method of a non-final class. > But if not caught in that block it becomes essentially impossible to actually find out what happened. Please elaborate on this. If the exception is not caught, you get a stack trace, so it's not completely clear to me what you are complaining about here.

admin commented

2026-01-22 15:42:06 +01:00

@pkly commented on GitHub (Mar 14, 2022):

I suppose it is, but this could be done without actually modifying the interface, with a comment + get_args, Symfony does that a lot. This example does cause a BC break, yes, but I was planning to implement the actual solution without it. It's just easier to show this way.

In our symfony application we noticed that whenever an exception is logged (to sentry) the stack trace (with entity manager closed exception) is just 2 blips, which are usually the symfony dispatcher and then just kernel.

Such information isn't really useful for anything, as it does not ultimately give any information about what caused the exception, only that the manager is closed at this point. If the previous exception was not logged (granted, it should be, but it can be difficult sometimes), no information will be available and debbuging is way harder.

The stack trace from errorIfClosed is always just Doctrine->errorIfClosed + your code up to flush/persist, which is not what caused it.

@pkly commented on GitHub (Mar 14, 2022): I suppose it is, but this could be done without actually modifying the interface, with a comment + get_args, Symfony does that a lot. This example does cause a BC break, yes, but I was planning to implement the actual solution without it. It's just easier to show this way. In our symfony application we noticed that whenever an exception is logged (to sentry) the stack trace (with entity manager closed exception) is just 2 blips, which are usually the symfony dispatcher and then just kernel. Such information isn't really useful for anything, as it does not ultimately give any information about what caused the exception, only that the manager is closed at this point. If the previous exception was not logged (granted, it should be, but it can be difficult sometimes), no information will be available and debbuging is way harder. The stack trace from errorIfClosed is always just Doctrine->errorIfClosed + your code up to flush/persist, which is not what caused it.

admin commented

2026-01-22 15:42:06 +01:00

@greg0ire commented on GitHub (Mar 14, 2022):

I suppose it is, but this could be done without actually modifying the interface, with a comment + get_args, Symfony does that a lot. This example does cause a BC break, yes, but I was planning to implement the actual solution without it. It's just easier to show this way.

I've done this myself in Doctrine, just wanted you to make the plan a bit more explicit.

One thing that is still unclear to me is where in Doctrine is there a call to close() that is not followed by throw $e? In other words, in what situation is the original exception you would like to get not available?

@greg0ire commented on GitHub (Mar 14, 2022): > I suppose it is, but this could be done without actually modifying the interface, with a comment + get_args, Symfony does that a lot. This example does cause a BC break, yes, but I was planning to implement the actual solution without it. It's just easier to show this way. I've done this myself in Doctrine, just wanted you to make the plan a bit more explicit. One thing that is still unclear to me is where in Doctrine is there a call to `close()` that is not followed by `throw $e`? In other words, in what situation is the original exception you would like to get not available?

admin commented

2026-01-22 15:42:06 +01:00

@pkly commented on GitHub (Mar 14, 2022):

It's not that it's not followed, but rather that the exception may not be necessairly caught by the programmer when flushing explicitly.

In our experience it is possible that a flush somewhere will trigger some events to then cause the entity manager to be closed, which will not be caught at that point in time (as it's for example being fired in an event dispatcher), and thus will be somewhat lost in the process. It's hard to exactly poipoint causes of those errors in our case, other than wrapping every flush into a try/catch block (some of which already are).

I've also made a patch for the orm version we're using and we're going to deploy it to our application in a while to see if it helps.

@pkly commented on GitHub (Mar 14, 2022): It's not that it's not followed, but rather that the exception may not be necessairly caught by the programmer when flushing explicitly. In our experience it is possible that _a_ flush somewhere will trigger some events to then cause the entity manager to be closed, which will not be caught at that point in time (as it's for example being fired in an event dispatcher), and thus will be somewhat lost in the process. It's hard to exactly poipoint causes of those errors in our case, other than wrapping every flush into a try/catch block (some of which already are). I've also made a patch for the orm version we're using and we're going to deploy it to our application in a while to see if it helps.

admin commented

2026-01-22 15:42:06 +01:00

@greg0ire commented on GitHub (Mar 14, 2022):

🤔 if an exception is not caught (as in, there is no try/catch block), then I fail to see why it would be lost… is there no default exception handler in the application you're mentioning that would log the exception somewhere? The situation you're describing makes it sound like the code you are dealing with looks like this:

try {
   $em->flush();
} catch (Throwable $e) {
    // nothing is done here
}

But it does not, right?

@greg0ire commented on GitHub (Mar 14, 2022): :thinking: if an exception is not caught (as in, there is no try/catch block), then I fail to see why it would be lost… is there no default exception handler in the application you're mentioning that would log the exception somewhere? The situation you're describing makes it sound like the code you are dealing with looks like this: ```php try { $em->flush(); } catch (Throwable $e) { // nothing is done here } ``` But it does not, right?

admin commented

2026-01-22 15:42:06 +01:00

@pkly commented on GitHub (Mar 15, 2022):

Seeing how our application is based on symfony there is one. It does appear to be skipped somehow however. I'm not exactly sure why, I have some guesses I suppose.

I can see how this could be seen as moving the responsibility of debugging onto the orm however, so if you're against this then I won't push. I just thought it could be useful for someone other than me.

@pkly commented on GitHub (Mar 15, 2022): Seeing how our application is based on symfony there is one. It does appear to be skipped somehow however. I'm not exactly sure why, I have some guesses I suppose. I can see how this could be seen as moving the responsibility of debugging onto the orm however, so if you're against this then I won't push. I just thought it could be useful for someone other than me.

admin commented

2026-01-22 15:42:06 +01:00

@greg0ire commented on GitHub (Mar 15, 2022):

if you're against this

I'm undecided, and I think you should try to fully understand the issue, so that we can decide whether it is legitimate or whether this would be a fix in the wrong place. Maybe set a breakpoint in close() to understand why this is happening?

@greg0ire commented on GitHub (Mar 15, 2022): > if you're against this I'm undecided, and I think you should try to fully understand the issue, so that we can decide whether it is legitimate or whether this would be a fix in the wrong place. Maybe set a breakpoint in `close()` to understand why this is happening?

admin commented

2026-01-22 15:42:07 +01:00

@pkly commented on GitHub (Mar 15, 2022):

Unfortunately it's not as easy for me to debug these occurrences since they happen on our production servers a few times a month.

@pkly commented on GitHub (Mar 15, 2022): Unfortunately it's not as easy for me to debug these occurrences since they happen on our production servers a few times a month.

admin commented

2026-01-22 15:42:07 +01:00

@greg0ire commented on GitHub (Mar 16, 2022):

Then I suggest you fork the ORM with your idea, to validate it on your production server before suggesting we push it to all the production servers of all our users. Don't hesitate to ask me to reopen once you've done that.

@greg0ire commented on GitHub (Mar 16, 2022): Then I suggest you fork the ORM with your idea, to validate it on your production server before suggesting we push it to all the production servers of all our users. Don't hesitate to ask me to reopen once you've done that.

admin commented

2026-01-22 15:42:08 +01:00

@pkly commented on GitHub (Mar 25, 2022):

@greg0ire coming back to this topic, after deploying the changes on our production servers we were able to find a few more bugs this way.
Since this is just a previous exception being traced it helps with using services like sentry.io, which can then pinpoint what exactly happened.

In our case this happens because we do a lot of tasks in symfony on termination. Some exceptions will not be caught, and I don't think they actually show up by default (for some reason), so at least in our scenario it was useful to pinpoint a few crashes so far.

Basically the flow was like the following:
Request -> Controller -> Service (crash) -> [Caught by kernel, not logged (our error)] -> Termination event -> subscriber -> crash (entity manager closed)

@pkly commented on GitHub (Mar 25, 2022): @greg0ire coming back to this topic, after deploying the changes on our production servers we were able to find a few more bugs this way. Since this is just a previous exception being traced it helps with using services like sentry.io, which can then pinpoint what exactly happened. In our case this happens because we do a lot of tasks in symfony on termination. Some exceptions will not be caught, and I don't think they actually show up by default (for some reason), so at least in our scenario it was useful to pinpoint a few crashes so far. Basically the flow was like the following: Request -> Controller -> Service (crash) -> [Caught by kernel, not logged (our error)] -> Termination event -> subscriber -> crash (entity manager closed)

admin commented

2026-01-22 15:42:08 +01:00

@greg0ire commented on GitHub (Mar 25, 2022):