-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
bug #12336 Fix timeout on 404 Not Found errors when using templating …
…and Symfony 4.4 (pamil) This PR was merged into the 1.9 branch. Discussion ---------- This issue happens when upgrading Sylius 1.8 to Sylius 1.9, while keeping Symfony 4.4. Commits ------- 310986e Fix timeout on 404 Not Found errors when using templating and Symfony 4.4
- Loading branch information
Showing
10 changed files
with
354 additions
and
11 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
40 changes: 40 additions & 0 deletions
40
...reBundle/DependencyInjection/Compiler/CircularDependencyBreakingExceptionListenerPass.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Sylius package. | ||
* | ||
* (c) Paweł Jędrzejewski | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Sylius\Bundle\CoreBundle\DependencyInjection\Compiler; | ||
|
||
use Sylius\Bundle\CoreBundle\EventListener\CircularDependencyBreakingExceptionListener; | ||
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; | ||
use Symfony\Component\DependencyInjection\ContainerBuilder; | ||
use Symfony\Component\DependencyInjection\Definition; | ||
use Symfony\Component\DependencyInjection\Reference; | ||
|
||
final class CircularDependencyBreakingExceptionListenerPass implements CompilerPassInterface | ||
{ | ||
/** @psalm-suppress MissingDependency */ | ||
public function process(ContainerBuilder $container): void | ||
{ | ||
if (!$container->has('twig.exception_listener')) { | ||
return; | ||
} | ||
|
||
$definition = new Definition(CircularDependencyBreakingExceptionListener::class); | ||
$definition->setDecoratedService('twig.exception_listener'); | ||
$definition->addArgument(new Reference(CircularDependencyBreakingExceptionListener::class .'.inner')); | ||
|
||
$container->setDefinition( | ||
CircularDependencyBreakingExceptionListener::class, | ||
$definition | ||
); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
96 changes: 96 additions & 0 deletions
96
src/Sylius/Bundle/CoreBundle/EventListener/CircularDependencyBreakingExceptionListener.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Sylius package. | ||
* | ||
* (c) Paweł Jędrzejewski | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Sylius\Bundle\CoreBundle\EventListener; | ||
|
||
use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; | ||
use Symfony\Component\HttpKernel\EventListener\ExceptionListener; | ||
|
||
/** | ||
* For Symfony 4. | ||
* | ||
* `Symfony\Component\HttpKernel\EventListener\ExceptionListener::onKernelException` happens to set previous | ||
* exception for a wrapper exception. This is meant to improve DX while debugging nested exceptions, | ||
* but also creates some issues. | ||
* | ||
* After upgrading to ResourceBundle v1.7 and GridBundle v1.8, the test suite started to fail | ||
* because of a timeout. By artifically setting the previous exception, in some cases it created | ||
* an exception with circular dependencies, so that: | ||
* | ||
* `$exception->getPrevious()->getPrevious()->getPrevious() === $exception` | ||
* | ||
* That exception is rethrown and other listeners like `Symfony\Component\Security\Http\Firewall\ExceptionListener` | ||
* try to deal with an exception and all their previous ones, causing infinite loops. | ||
* | ||
* This fix only works if "framework.templating" setting DOES NOT include "twig". Otherwise, TwigBundle | ||
* registers deprecated `Symfony\Component\HttpKernel\EventListener\ExceptionListener`, removes the non-deprecated | ||
* "exception_listener" service, so that the issue still persists. | ||
* | ||
* This listener behaves as a decorator, but also extends the original ExceptionListener, because of yet another | ||
* listener `ApiPlatform\Core\EventListener\ExceptionListener` requires the original class. | ||
* | ||
* @internal | ||
* | ||
* @see \Symfony\Component\HttpKernel\EventListener\ExceptionListener | ||
* | ||
* @psalm-suppress UndefinedClass | ||
* @psalm-suppress MissingDependency | ||
*/ | ||
final class CircularDependencyBreakingExceptionListener extends ExceptionListener | ||
{ | ||
/** @var ExceptionListener */ | ||
private $decoratedListener; | ||
|
||
public function __construct(ExceptionListener $decoratedListener) | ||
{ | ||
$this->decoratedListener = $decoratedListener; | ||
} | ||
|
||
public function logKernelException(GetResponseForExceptionEvent $event): void | ||
{ | ||
$this->decoratedListener->logKernelException($event); | ||
} | ||
|
||
public function onKernelException(GetResponseForExceptionEvent $event): void | ||
{ | ||
try { | ||
$this->decoratedListener->onKernelException($event); | ||
} catch (\Throwable $throwable) { | ||
$this->breakCircularDependency($throwable); | ||
|
||
throw $throwable; | ||
} | ||
} | ||
|
||
private function breakCircularDependency(\Throwable $throwable): void | ||
{ | ||
$throwables = []; | ||
|
||
do { | ||
$throwables[] = $throwable; | ||
|
||
if (in_array($throwable->getPrevious(), $throwables, true)) { | ||
$this->removePreviousFromThrowable($throwable); | ||
} | ||
|
||
$throwable = $throwable->getPrevious(); | ||
} while (null !== $throwable); | ||
} | ||
|
||
private function removePreviousFromThrowable(\Throwable $throwable): void | ||
{ | ||
$previous = new \ReflectionProperty($throwable instanceof \Exception ? \Exception::class : \Error::class, 'previous'); | ||
$previous->setAccessible(true); | ||
$previous->setValue($throwable, null); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
45 changes: 45 additions & 0 deletions
45
...eBundle/Tests/DependencyInjection/CircularDependencyBreakingExceptionListenerPassTest.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Sylius package. | ||
* | ||
* (c) Paweł Jędrzejewski | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Sylius\Bundle\CoreBundle\Tests\DependencyInjection; | ||
|
||
use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractCompilerPassTestCase; | ||
use Sylius\Bundle\CoreBundle\DependencyInjection\Compiler\CircularDependencyBreakingExceptionListenerPass; | ||
use Symfony\Component\DependencyInjection\ContainerBuilder; | ||
use Symfony\Component\DependencyInjection\Definition; | ||
use Sylius\Bundle\CoreBundle\EventListener\CircularDependencyBreakingExceptionListener; | ||
|
||
final class CircularDependencyBreakingExceptionListenerPassTest extends AbstractCompilerPassTestCase | ||
{ | ||
|
||
public function it_register_circular_dependency_breaking_error_listener_when_exception_listener_is_registered(): void | ||
{ | ||
$this->container->setDefinition('twig.exception_listener', new Definition('ExceptionListener')); | ||
|
||
$this->compile(); | ||
|
||
$this->assertContainerBuilderHasService(CircularDependencyBreakingExceptionListener::class); | ||
} | ||
|
||
public function it_does_nothing_when_exception_listener_is_not_registered(): void | ||
{ | ||
$this->compile(); | ||
|
||
$this->assertContainerBuilderNotHasService(CircularDependencyBreakingExceptionListener::class); | ||
} | ||
|
||
protected function registerCompilerPass(ContainerBuilder $container): void | ||
{ | ||
$container->addCompilerPass(new CircularDependencyBreakingExceptionListenerPass()); | ||
} | ||
} |
Oops, something went wrong.