Make the RequestIntegration integration fetch the request from the request stack (#361)

Author: ste93cry

.github/workflows/tests.yaml

@@ -53,7 +53,7 @@ jobs:
           php-version: ${{ matrix.php }}
           coverage: xdebug
       - run: |
-          sed -ri '/symfony\/(monolog-bundle|phpunit-bridge|messenger)/! s/"symfony\/(.+)": "(.+)"/"symfony\/\1": "'${{ matrix.symfony_constraint }}'"/' composer.json;
+          sed -ri '/symfony\/(monolog-bundle|phpunit-bridge|messenger|psr-http-message-bridge)/! s/"symfony\/(.+)": "(.+)"/"symfony\/\1": "'${{ matrix.symfony_constraint }}'"/' composer.json;
         if: matrix.symfony_constraint
       - run: composer remove --dev symfony/messenger --no-update
         if: matrix.symfony_constraint == '3.4.*' || matrix.composer_option == '--prefer-lowest'

CHANGELOG.md

@@ -11,6 +11,7 @@
 - [BC BREAK] Refactorized the configuration tree and the definitions of some container services (#401)
 - Support the XML format for the bundle configuration (#401)
 - PHP 8 support (#399, thanks to @Yozhef)
+- Retrieve the request from the `RequestStack` when using the `RequestIntegration` integration (#361)
 
 ## 3.5.3 (2020-10-13)
 

composer.json

@@ -28,6 +28,7 @@
         "symfony/dependency-injection": "^3.4.43||^4.4.11||^5.0.11",
         "symfony/event-dispatcher": "^3.4.43||^4.4.11||^5.0.11",
         "symfony/http-kernel": "^3.4.43||^4.4.11||^5.0.11",
+        "symfony/psr-http-message-bridge": "^2.0",
         "symfony/security-core": "^3.4.43||^4.4.11||^5.0.11"
     },
     "require-dev": {

src/DependencyInjection/SentryExtension.php

@@ -9,6 +9,9 @@
 use Sentry\Client;
 use Sentry\ClientBuilder;
 use Sentry\Integration\IgnoreErrorsIntegration;
+use Sentry\Integration\IntegrationInterface;
+use Sentry\Integration\RequestFetcherInterface;
+use Sentry\Integration\RequestIntegration;
 use Sentry\Monolog\Handler;
 use Sentry\Options;
 use Sentry\SentryBundle\EventListener\ErrorListener;
@@ -98,7 +101,7 @@ private function registerConfiguration(ContainerBuilder $container, array $confi
         }
 
         if (isset($options['integrations'])) {
-            $options['integrations'] = $this->configureIntegrationsOption($options['integrations'], $config['register_error_listener']);
+            $options['integrations'] = $this->configureIntegrationsOption($options['integrations'], $config);
         }
 
         $container
@@ -176,17 +179,30 @@ private function registerMonologHandlerConfiguration(ContainerBuilder $container
 
     /**
      * @param string[] $integrations
+     * @param array<string, mixed> $config
      *
      * @return array<Reference|Definition>
      */
-    private function configureIntegrationsOption(array $integrations, bool $registerErrorListener): array
+    private function configureIntegrationsOption(array $integrations, array $config): array
     {
-        $existsIgnoreErrorsIntegration = in_array(IgnoreErrorsIntegration::class, $integrations, true);
         $integrations = array_map(static function (string $value): Reference {
             return new Reference($value);
         }, $integrations);
 
-        if ($registerErrorListener && false === $existsIgnoreErrorsIntegration) {
+        $integrations = $this->configureErrorListenerIntegration($integrations, $config['register_error_listener']);
+        $integrations = $this->configureRequestIntegration($integrations, $config['options']['default_integrations'] ?? true);
+
+        return $integrations;
+    }
+
+    /**
+     * @param array<Reference|Definition> $integrations
+     *
+     * @return array<Reference|Definition>
+     */
+    private function configureErrorListenerIntegration(array $integrations, bool $registerErrorListener): array
+    {
+        if ($registerErrorListener && ! $this->isIntegrationEnabled(IgnoreErrorsIntegration::class, $integrations)) {
             // Prepend this integration to the beginning of the array so that
             // we can save some performance by skipping the rest of the integrations
             // if the error must be ignored
@@ -195,4 +211,37 @@ private function configureIntegrationsOption(array $integrations, bool $register
 
         return $integrations;
     }
+
+    /**
+     * @param array<Reference|Definition> $integrations
+     *
+     * @return array<Reference|Definition>
+     */
+    private function configureRequestIntegration(array $integrations, bool $useDefaultIntegrations): array
+    {
+        if ($useDefaultIntegrations && ! $this->isIntegrationEnabled(RequestIntegration::class, $integrations)) {
+            $integrations[] = new Definition(RequestIntegration::class, [new Reference(RequestFetcherInterface::class)]);
+        }
+
+        return $integrations;
+    }
+
+    /**
+     * @param class-string<IntegrationInterface> $integrationClass
+     * @param array<Reference|Definition> $integrations
+     */
+    private function isIntegrationEnabled(string $integrationClass, array $integrations): bool
+    {
+        foreach ($integrations as $integration) {
+            if ($integration instanceof Reference && $integrationClass === (string) $integration) {
+                return true;
+            }
+
+            if ($integration instanceof Definition && $integrationClass === $integration->getClass()) {
+                return true;
+            }
+        }
+
+        return false;
+    }
 }

src/Integration/RequestFetcher.php

@@ -0,0 +1,54 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Sentry\SentryBundle\Integration;
+
+use Psr\Http\Message\ServerRequestInterface;
+use Sentry\Integration\RequestFetcherInterface;
+use Symfony\Bridge\PsrHttpMessage\HttpMessageFactoryInterface;
+use Symfony\Component\HttpFoundation\RequestStack;
+
+/**
+ * This class fetches the server request from the request stack and converts it
+ * into a PSR-7 request that is suitable to be used by the {@see \Sentry\Integration\RequestIntegration}
+ * integration.
+ */
+final class RequestFetcher implements RequestFetcherInterface
+{
+    /**
+     * @var RequestStack The request stack
+     */
+    private $requestStack;
+
+    /**
+     * @var HttpMessageFactoryInterface The factory to convert Symfony requests to PSR-7 requests
+     */
+    private $httpMessageFactory;
+
+    /**
+     * Class constructor.
+     *
+     * @param RequestStack                $requestStack       The request stack
+     * @param HttpMessageFactoryInterface $httpMessageFactory The factory to convert Symfony requests to PSR-7 requests
+     */
+    public function __construct(RequestStack $requestStack, HttpMessageFactoryInterface $httpMessageFactory)
+    {
+        $this->requestStack = $requestStack;
+        $this->httpMessageFactory = $httpMessageFactory;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function fetchRequest(): ?ServerRequestInterface
+    {
+        $request = $this->requestStack->getCurrentRequest();
+
+        if (null === $request) {
+            return null;
+        }
+
+        return $this->httpMessageFactory->createRequest($request);
+    }
+}

src/Resources/config/services.xml

@@ -66,5 +66,33 @@
         <service id="Sentry\Monolog\Handler" class="Sentry\Monolog\Handler">
             <argument type="service" id="Sentry\State\HubInterface" />
         </service>
+
+        <service id="Sentry\Integration\RequestFetcherInterface" class="Sentry\SentryBundle\Integration\RequestFetcher">
+            <argument type="service" id="Symfony\Component\HttpFoundation\RequestStack" />
+            <argument type="service">
+                <service class="Symfony\Bridge\PsrHttpMessage\Factory\PsrHttpFactory">
+                    <argument type="service">
+                        <service class="Psr\Http\Message\ServerRequestFactoryInterface">
+                            <factory class="Http\Discovery\Psr17FactoryDiscovery" method="findServerRequestFactory" />
+                        </service>
+                    </argument>
+                    <argument type="service">
+                        <service class="Psr\Http\Message\StreamFactoryInterface">
+                            <factory class="Http\Discovery\Psr17FactoryDiscovery" method="findStreamFactory" />
+                        </service>
+                    </argument>
+                    <argument type="service">
+                        <service class="Psr\Http\Message\UploadedFileFactoryInterface">
+                            <factory class="Http\Discovery\Psr17FactoryDiscovery" method="findUploadedFileFactory" />
+                        </service>
+                    </argument>
+                    <argument type="service">
+                        <service class="Psr\Http\Message\ResponseFactoryInterface">
+                            <factory class="Http\Discovery\Psr17FactoryDiscovery" method="findResponseFactory" />
+                        </service>
+                    </argument>
+                </service>
+            </argument>
+        </service>
     </services>
 </container>

test/DependencyInjection/Fixtures/php/ignore_errors_integration_overridden.php

@@ -8,7 +8,6 @@
 $container->loadFromExtension('sentry', [
     'options' => [
         'integrations' => [
-            'App\\Sentry\\Integration\\FooIntegration',
             'Sentry\\Integration\\IgnoreErrorsIntegration',
         ],
     ],

test/DependencyInjection/Fixtures/xml/ignore_errors_integration_overridden.xml

@@ -8,7 +8,6 @@
 
     <sentry:config>
         <sentry:options>
-            <sentry:integration>App\Sentry\Integration\FooIntegration</sentry:integration>
             <sentry:integration>Sentry\Integration\IgnoreErrorsIntegration</sentry:integration>
         </sentry:options>
     </sentry:config>

test/DependencyInjection/Fixtures/yml/ignore_errors_integration_overridden.yml

@@ -1,5 +1,4 @@
 sentry:
     options:
         integrations:
-            - App\Sentry\Integration\FooIntegration
             - Sentry\Integration\IgnoreErrorsIntegration

test/DependencyInjection/SentryExtensionTest.php

@@ -72,10 +72,8 @@ public function errorListenerDataProvider(): \Generator
     public function testErrorListenerIsRemovedWhenDisabled(): void
     {
         $container = $this->createContainerFromFixture('error_listener_disabled');
-        $optionsDefinition = $container->getDefinition('sentry.client.options');
 
         $this->assertFalse($container->hasDefinition(ErrorListener::class));
-        $this->assertSame([], $optionsDefinition->getArgument(0)['integrations']);
     }
 
     /**
@@ -369,13 +367,20 @@ public function testErrorTypesOptionIsParsedFromStringToIntegerValue(): void
     public function testIgnoreErrorsIntegrationIsNotAddedTwiceIfAlreadyConfigured(): void
     {
         $container = $this->createContainerFromFixture('ignore_errors_integration_overridden');
-        $optionsDefinition = $container->getDefinition('sentry.client.options');
-        $expectedIntegrations = [
-            new Reference('App\\Sentry\\Integration\\FooIntegration'),
-            new Reference(IgnoreErrorsIntegration::class),
-        ];
+        $integrations = $container->getDefinition('sentry.client.options')->getArgument(0)['integrations'];
+        $ignoreErrorsIntegrationsCount = 0;
+
+        foreach ($integrations as $integration) {
+            if ($integration instanceof Reference && IgnoreErrorsIntegration::class === (string) $integration) {
+                ++$ignoreErrorsIntegrationsCount;
+            }
+
+            if ($integration instanceof Definition && IgnoreErrorsIntegration::class === $integration->getClass()) {
+                ++$ignoreErrorsIntegrationsCount;
+            }
+        }
 
-        $this->assertEquals($expectedIntegrations, $optionsDefinition->getArgument(0)['integrations']);
+        $this->assertSame(1, $ignoreErrorsIntegrationsCount);
     }
 
     private function createContainerFromFixture(string $fixtureFile): ContainerBuilder

test/Integration/RequestFetcherTest.php

@@ -0,0 +1,72 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Sentry\SentryBundle\Test\Integration;
+
+use GuzzleHttp\Psr7\ServerRequest;
+use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\TestCase;
+use Psr\Http\Message\ServerRequestInterface;
+use Sentry\SentryBundle\Integration\RequestFetcher;
+use Symfony\Bridge\PsrHttpMessage\HttpMessageFactoryInterface;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
+
+final class RequestFetcherTest extends TestCase
+{
+    /**
+     * @var RequestStack&MockObject
+     */
+    private $requestStack;
+
+    /**
+     * @var HttpMessageFactoryInterface&MockObject
+     */
+    private $httpMessageFactory;
+
+    /**
+     * @var RequestFetcher
+     */
+    private $requestFetcher;
+
+    protected function setUp(): void
+    {
+        $this->requestStack = $this->createMock(RequestStack::class);
+        $this->httpMessageFactory = $this->createMock(HttpMessageFactoryInterface::class);
+        $this->requestFetcher = new RequestFetcher($this->requestStack, $this->httpMessageFactory);
+    }
+
+    /**
+     * @dataProvider fetchRequestDataProvider
+     */
+    public function testFetchRequest(?Request $request, ?ServerRequestInterface $expectedRequest): void
+    {
+        $this->requestStack->expects($this->once())
+            ->method('getCurrentRequest')
+            ->willReturn($request);
+
+        $this->httpMessageFactory->expects(null !== $expectedRequest ? $this->once() : $this->never())
+            ->method('createRequest')
+            ->with($request)
+            ->willReturn($expectedRequest);
+
+        $this->assertSame($expectedRequest, $this->requestFetcher->fetchRequest());
+    }
+
+    /**
+     * @return \Generator<mixed>
+     */
+    public function fetchRequestDataProvider(): \Generator
+    {
+        yield [
+            null,
+            null,
+        ];
+
+        yield [
+            Request::create('http://www.example.com'),
+            new ServerRequest('GET', 'http://www.example.com'),
+        ];
+    }
+}