Fix #243: stop swallowing exceptions

Author: meyerbaptiste

Controller/ExceptionController.php

@@ -0,0 +1,65 @@
+<?php
+
+/*
+ * This file is part of the DunglasApiBundle package.
+ *
+ * (c) Kévin Dunglas <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Dunglas\ApiBundle\Controller;
+
+use Dunglas\ApiBundle\JsonLd\Response;
+use Symfony\Component\Debug\Exception\FlattenException;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpKernel\Log\DebugLoggerInterface;
+use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
+
+/**
+ * Render a normalized exception for a given {@see \Symfony\Component\Debug\Exception\FlattenException}.
+ *
+ * @author Baptiste Meyer <[email protected]>
+ */
+class ExceptionController
+{
+    /**
+     * @var NormalizerInterface
+     */
+    private $normalizer;
+
+    /**
+     * @param NormalizerInterface $normalizer
+     */
+    public function __construct(NormalizerInterface $normalizer)
+    {
+        $this->normalizer = $normalizer;
+    }
+
+    /**
+     * Converts a {@see \Symfony\Component\Debug\Exception\FlattenException}
+     * to a {@see \Dunglas\ApiBundle\JsonLd\Response}.
+     *
+     * @param Request                   $request
+     * @param FlattenException          $exception
+     * @param DebugLoggerInterface|null $logger
+     *
+     * @return Response
+     */
+    public function showAction(Request $request, FlattenException $exception, DebugLoggerInterface $logger = null)
+    {
+        if (
+            'Dunglas\ApiBundle\Exception\DeserializationException' === $exception->getClass()
+            || is_subclass_of($exception->getClass(), 'Dunglas\ApiBundle\Exception\DeserializationException')
+        ) {
+            $exception->setStatusCode(Response::HTTP_BAD_REQUEST);
+        }
+
+        return new Response(
+            $this->normalizer->normalize($exception, 'hydra-error'),
+            $exception->getStatusCode(),
+            $exception->getHeaders()
+        );
+    }
+}

DependencyInjection/Compiler/TwigExceptionListenerPass.php

@@ -0,0 +1,33 @@
+<?php
+
+/*
+ * This file is part of the DunglasApiBundle package.
+ *
+ * (c) Kévin Dunglas <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Dunglas\ApiBundle\DependencyInjection\Compiler;
+
+use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+
+/**
+ * Remove the definition of the "twig.exception_listener" service when Twig is enabled.
+ *
+ * @author Baptiste Meyer <[email protected]>
+ */
+class TwigExceptionListenerPass implements CompilerPassInterface
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function process(ContainerBuilder $container)
+    {
+        if ($container->has('api.hydra.listener.exception') && $container->has('twig.exception_listener')) {
+            $container->removeDefinition('twig.exception_listener');
+        }
+    }
+}

DunglasApiBundle.php

@@ -13,6 +13,7 @@
 
 use Dunglas\ApiBundle\DependencyInjection\Compiler\DataProviderPass;
 use Dunglas\ApiBundle\DependencyInjection\Compiler\ResourcePass;
+use Dunglas\ApiBundle\DependencyInjection\Compiler\TwigExceptionListenerPass;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\HttpKernel\Bundle\Bundle;
 
@@ -32,5 +33,6 @@ public function build(ContainerBuilder $container)
 
         $container->addCompilerPass(new ResourcePass());
         $container->addCompilerPass(new DataProviderPass());
+        $container->addCompilerPass(new TwigExceptionListenerPass());
     }
 }

Hydra/EventListener/RequestExceptionListener.php

@@ -11,11 +11,13 @@
 
 namespace Dunglas\ApiBundle\Hydra\Serializer;
 
+use Symfony\Component\Debug\Exception\FlattenException;
 use Symfony\Component\Routing\RouterInterface;
 use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
 
 /**
- * Converts {@see \Exception} to a Hydra error representation.
+ * Converts {@see \Exception} or {@see \Symfony\Component\Debug\Exception\FlattenException}
+ * to a Hydra error representation.
  *
  * @author Kévin Dunglas <[email protected]>
  * @author Samuel ROZE <[email protected]>
@@ -46,7 +48,7 @@ public function __construct(RouterInterface $router, $debug)
      */
     public function normalize($object, $format = null, array $context = array())
     {
-        if ($object instanceof \Exception) {
+        if ($object instanceof \Exception || $object instanceof FlattenException) {
             $message = $object->getMessage();
 
             if ($this->debug) {
@@ -73,6 +75,6 @@ public function normalize($object, $format = null, array $context = array())
      */
     public function supportsNormalization($data, $format = null)
     {
-        return 'hydra-error' === $format && $data instanceof \Exception;
+        return 'hydra-error' === $format && ($data instanceof \Exception || $data instanceof FlattenException);
     }
 }

Hydra/Serializer/ErrorNormalizer.php

@@ -14,6 +14,12 @@
             <argument>%api.description%</argument>
         </service>
 
+        <!-- Controllers -->
+
+        <service id="api.hydra.controller.exception" class="Dunglas\ApiBundle\Controller\ExceptionController">
+            <argument type="service" id="api.hydra.normalizer.error" />
+        </service>
+
         <!-- Event listeners -->
 
         <service id="api.hydra.listener.link_header_response" class="Dunglas\ApiBundle\Hydra\EventListener\LinkHeaderResponseListener">
@@ -22,10 +28,12 @@
             <tag name="kernel.event_listener" event="kernel.response" method="onKernelResponse" />
         </service>
 
-        <service id="api.hydra.listener.request_exception" class="Dunglas\ApiBundle\Hydra\EventListener\RequestExceptionListener">
-            <argument type="service" id="api.hydra.normalizer.error" />
+        <service id="api.hydra.listener.exception" class="Symfony\Component\HttpKernel\EventListener\ExceptionListener">
+            <argument>api.hydra.controller.exception:showAction</argument>
+            <argument type="service" id="logger" on-invalid="null" />
 
-            <tag name="kernel.event_listener" event="kernel.exception" method="onKernelException" />
+            <tag name="kernel.event_subscriber" />
+            <tag name="monolog.logger" channel="request" />
         </service>
 
         <!-- Serializer -->

Resources/config/hydra.xml

@@ -0,0 +1,33 @@
+<?php
+
+/*
+ * This file is part of the DunglasApiBundle package.
+ *
+ * (c) Kévin Dunglas <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Dunglas\ApiBundle\Tests\DependencyInjection\Compiler;
+
+use Dunglas\ApiBundle\DependencyInjection\Compiler\TwigExceptionListenerPass;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+
+/**
+ * @author Baptiste Meyer <[email protected]>
+ */
+class TwigExceptionListenerPassTest extends \PHPUnit_Framework_TestCase
+{
+    public function testProcess()
+    {
+        $containerBuilder = new ContainerBuilder();
+        $containerBuilder->register('twig.exception_listener');
+        $containerBuilder->register('api.hydra.listener.exception');
+
+        (new TwigExceptionListenerPass())->process($containerBuilder);
+
+        $this->assertFalse($containerBuilder->hasDefinition('twig.exception_listener'));
+        $this->assertTrue($containerBuilder->hasDefinition('api.hydra.listener.exception'));
+    }
+}

Tests/DependencyInjection/Compiler/TwigExceptionListenerPassTest.php

@@ -174,8 +174,9 @@ private function getContainerBuilderProphecy($withDoctrine = true)
         $containerBuilderProphecy->setDefinition('api.json_ld.normalizer.datetime', $definitionArgument)->shouldBeCalled();
         $containerBuilderProphecy->setDefinition('api.json_ld.encoder', $definitionArgument)->shouldBeCalled();
         $containerBuilderProphecy->setDefinition('api.hydra.documentation_builder', $definitionArgument)->shouldBeCalled();
+        $containerBuilderProphecy->setDefinition('api.hydra.controller.exception', $definitionArgument)->shouldBeCalled();
         $containerBuilderProphecy->setDefinition('api.hydra.listener.link_header_response', $definitionArgument)->shouldBeCalled();
-        $containerBuilderProphecy->setDefinition('api.hydra.listener.request_exception', $definitionArgument)->shouldBeCalled();
+        $containerBuilderProphecy->setDefinition('api.hydra.listener.exception', $definitionArgument)->shouldBeCalled();
         $containerBuilderProphecy->setDefinition('api.hydra.normalizer.collection', $definitionArgument)->shouldBeCalled();
         $containerBuilderProphecy->setDefinition('api.hydra.normalizer.constraint_violation_list', $definitionArgument)->shouldBeCalled();
         $containerBuilderProphecy->setDefinition('api.hydra.normalizer.error', $definitionArgument)->shouldBeCalled();