Possible new HttpKernel event? #64842

The important part is keeping the default behavior unchanged and making any caching opt-in or explicitly supported by resolvers, because automatically caching arbitrary resolver output could introduce subtle behavior changes.

That could probably be a toggle or something, or just have the default flow through this event and generally be non-repeatable by default.
Since this would be a new event of sorts, you wouldn't have any consumers for it (other than the symfony one, which would trigger the callback), so it would be safe to add and would not change any behavior in existing applications.

Making the listener call a callable that triggers the whole controller handling logic is a no-go to me: this would call the controller multiple times when you add multiple listeners on that even that work this way. This is clearly not a proper API for an event-based extension point.

I'm open to suggestions as for how you'd solve it, but honestly I don't really see anything that would allow us to repeat an action other than making it simply a callback.

edit: actually, it would be possible, but it'd be still rather... weird?

$maxAttempts = 3; // preferably configurable from symfony's config
$attempt = 0;

while (true) {
        $event = new ControllerDispatchStartEvent();
        $this->dispatcher->dispatch($event);

        // possible add exit point via $event->getResponse?

        // continue as normal
        // controller arguments
        $arguments = $this->argumentResolver->getArguments($request, $controller, $event->getControllerReflector());

        $event = new ControllerArgumentsEvent($this, $event, $arguments, $request, $type);
        $this->dispatcher->dispatch($event, KernelEvents::CONTROLLER_ARGUMENTS);
        $controller = $event->getController();
        $arguments = $event->getArguments();

        // call controller
        try {
            $response = $controller(...$arguments);
        } catch (\Throwable $e) {
            $event = new ControllerDispatchExceptionEvent($e);
            $this->dispatcher->dispatch($event);

            if ($attempt >= $maxAttempts) {
               throw $e;
            }

            if ($event->shouldRetry()) {
               $attempt++;

               $event = new ControllerRetryEvent();
               $this->dispatcher->dispatch($event);
               continue;
            }
        }

        $event = new ControllerDispatchFinishEvent($response);
        $this-dispatcher->dispatch($event);

        if ($event->shouldRetry()) {
               $attempt++;

               $event = new ControllerRetryEvent();
               $this->dispatcher->dispatch($event);
               continue;
        }

        $response = $event->getResponse(); // would just default back to itself
        break;
}

something like this, maybe?

I think this approach actually avoids the biggest issue with the callback idea.

The good part is that HttpKernel still owns the execution flow. Listeners are only deciding whether something should be retried, instead of manually calling the controller execution themselves.

It also solves the main problem with transactions: on retry, the whole flow can run again:

• resolve controller arguments
• run CONTROLLER_ARGUMENTS
• execute the controller

So things like #[MapEntity] or custom argument resolvers would get a fresh resolution instead of reusing stale objects from the previous attempt.

The main thing to be careful about is resetting state between attempts. A retry should not reuse anything from the previous run that may have changed, such as resolved arguments or request-scoped state.

I agree that a callback is the simplest way to represent a repeatable action, but this event-driven approach feels more Symfony-like because the kernel keeps control of when the controller is executed.

The main question would be whether retries should be a responsibility of HttpKernel itself or a separate layer built on top of this lifecycle.

Yeah, but this would not be aware of possible caches from argument resolvers, which again, could be a separate feature (with an optional interface which would be later required).

The only thing here is that the request should be copied from before it's mutated the first time, so we could keep re-using the fresh one (at least for the time being).

I believe this solution above would be sufficient to achieve the results we're aiming for, but again, this is just a proposal, to see if something like this would even be accepted. I believe there is a need to be able to transparently retry the controller request, currently we just do simple resolution in the arguments (usually just resolving to doctrine references and then re-loading the entities inside of a transaction with locks).

agree, the request state and resolver caching are probably the two areas that need the most attention.

keeping a copy of the original request before any mutation happens sounds like a practical first step. That would at least ensure that every retry starts from the same input state instead of carrying changes from the previous attempt.

the resolver cache problem feels like a separate concern though. A retry mechanism probably should not need to know about every possible resolver implementation. If argument resolvers need caching support in the future, an explicit contract/interface would make more sense so symfony can decide what can be reused safely

the important part of this proposal is that it creates a proper retry boundary around the whole controller lifecycle. Currently, applications have to work around this by resolving arguments first and then manually reloading things inside transactions, which is easy to get wrong.

Having an opt-in way to retry the complete flow transparently would make cases like Doctrine entity loading with locks much cleaner.

The cachable argument resolving could just be done strictly in the ArgumentResolver, so nothing in the kernel would need to know about it - the only issue is that there would need to be extra cache for a short while (could probably be cleared after the response is ready to be served).

As for the opt-in retries - I don't believe this would be symfony's responsibility? We have a bundle which provides a retryable service (wrapping around a callback, which is why my first idea was that), and we'd probably just hook into these events to handle it.

Maybe also add some sort of route marker, that we wish to actually start a transaction (not always needed) (#[WithTransaction] on the method in controller), but again, this would be outside of symfony's scope.

What good would forwarding do here? I'm not sure exactly, what you'd just forward the same request again?

So, you start a transaction on kernel.controller and then just run the controller inside of the transaction through decorating it with kernel.controller_arguments, but where's the possibility of a restart? All you added was a transaction, which you can just be done by starting it on the controller event and ending it on a response event.

But this is not what I'm after at all.

Yes forwarding would allow to “replay” the request inside of the same process. Not sure this would work but the idea feels like a starting point.

where's the possibility of a restart? All you added was a transaction

You can also see the original controller call being wrapped in a try/catch. That would be your way of knowing something wrong happened.

Yeah but knowing something wrong happened is not useful. Symfony already sends an exception event if an exception is thrown in the controller.

Since everything I say is wrong or not useful I’ll let you figure what you need by yourself 👋

I'm not trying to be rude or anything, I just don't think those solutions would help in achieving the effect I'm hoping for.

• Restarting the transaction without reloading the arguments may hold stale data/entities (crash)
• Knowing that an exception occurred inside of a controller is not very useful, if you're trying to be able to "restart" it, as that knowledge alone is not enough since you don't control the controller entrypoint (see above, stale arguments)
• Symfony already produces an exception event if an exception occurs inside of the controller
• Restarting transactions inside of a controller is already possible, but that assumes the transaction (and data) is loaded inside of the controller function (extra boilerplate and repeating code, we already do this)