Skip to content

fix(symfony): ensure ErrorListener is fully stateless to prevent stat…#7921

Open
KevinMartinsDev wants to merge 1 commit intoapi-platform:4.3from
KevinMartinsDev:fix/error-listener-stateless
Open

fix(symfony): ensure ErrorListener is fully stateless to prevent stat…#7921
KevinMartinsDev wants to merge 1 commit intoapi-platform:4.3from
KevinMartinsDev:fix/error-listener-stateless

Conversation

@KevinMartinsDev
Copy link
Copy Markdown

Description

This PR is the third part of the worker mode compatibility audit (see #7918). It refactors the ErrorListener to be strictly stateless.

The Issue

The ErrorListener previously stored the target controller in a private property during the request duplication process. In persistent memory runtimes (FrankenPHP, Swoole, etc.):

  1. State Leakage: If an error occurred, the $controller property was mutated. If not properly reset (and it wasn't), the next error handled by the same worker could inherit the previous request's controller.
  2. Thread Safety: Mutating service properties is unsafe in multi-threaded PHP environments.

The Solution

The listener has been refactored to remove the mutable $controller property entirely:

  • Stateless logic: The choice of controller (API Platform's error handler vs. Symfony's default) is now determined locally within duplicateRequest.
  • Parent Fallback: We now explicitly call parent::duplicateRequest($exception, $request) when we need to delegate to Symfony, without modifying the listener's internal state.
  • BC Compliance: Since the property was private, removing it does not break the public API. The constructor signature remains unchanged for backward compatibility.

This ensures that every error handled by the worker starts with a "clean slate", regardless of what happened in previous requests.

@soyuka
Copy link
Copy Markdown
Member

soyuka commented Apr 27, 2026

Suggestion:

diff --git a/src/Symfony/EventListener/ErrorListener.php b/src/Symfony/EventListener/ErrorListener.php
index 9b5849bc133..b2c0ab8a000 100644
--- a/src/Symfony/EventListener/ErrorListener.php
+++ b/src/Symfony/EventListener/ErrorListener.php
@@ -80,14 +80,8 @@ final class ErrorListener extends SymfonyErrorListener

         // Let the error handler take this we don't handle HTML nor non-api platform requests
         if (false === ($apiOperation?->getExtraProperties()['_api_error_handler'] ?? true) || 'html' === $format) {
-            if (null === $this->controller) {
-                $dup = $request->duplicate(null, null, ['_controller' => 'error_controller', 'exception' => $exception]);
-                $dup->setMethod('GET');
-
-                return $dup;
-            }
-
             $dup = parent::duplicateRequest($exception, $request);
+            // Override parent's `_controller` attribute without mutating $this->controller (worker-mode safety).
             $dup->attributes->set('_controller', 'error_controller');

             return $dup;
@@ -98,12 +92,6 @@ final class ErrorListener extends SymfonyErrorListener
         }

         $dup = parent::duplicateRequest($exception, $request);
-
-        if ($dup === $request) {
-            $dup = $request->duplicate(null, null, ['exception' => $exception]);
-            $dup->setMethod('GET');
-        }
-
         $operation = $this->initializeExceptionOperation($request, $exception, $format, $apiOperation);

         if (null === $operation->getProvider()) {

You can git apply this, also please target 4.3

@KevinMartinsDev KevinMartinsDev force-pushed the fix/error-listener-stateless branch from 685d43a to a068b51 Compare April 27, 2026 14:00
@KevinMartinsDev KevinMartinsDev changed the base branch from main to 4.3 April 27, 2026 14:01
@KevinMartinsDev
Copy link
Copy Markdown
Author

Done 👍 @soyuka


$refl = new \ReflectionClass($errorListener);
$controllerProp = $refl->getProperty('controller');
$controllerProp->setAccessible(true);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see ci reports this is a mistake

@KevinMartinsDev KevinMartinsDev force-pushed the fix/error-listener-stateless branch from a068b51 to a9ab663 Compare April 28, 2026 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants