Fix fatal error when the SERVER_PROTOCOL header is missing (#495)

getsentry · May 5, 2021 · c016dc2 · c016dc2
1 parent ff0c7a3
commit c016dc2
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 - Fix the conditions to automatically enable the cache instrumentation when possible (#487)
 - Fix deprecations triggered by Symfony 5.3 (#489)
+- Fix fatal error when the `SERVER_PROTOCOL` header is missing (#495)
 
 ## 4.1.0 (2021-04-19)
 
@@ -13,7 +14,7 @@
 - Add support for distributed tracing of SQL queries while using Doctrine DBAL (#426)
 - Add support for distributed tracing when running a console command (#455)
 - Add support for distributed tracing of cache pools (#477)
-- Add `Full command` to extras for CLI commands, which includes command with all arguments
+- Add the full CLI command string to the extra context (#352)
 - Deprecate the `Sentry\SentryBundle\EventListener\ConsoleCommandListener` class in favor of its parent class `Sentry\SentryBundle\EventListener\ConsoleListener` (#429)
 - Lower the required version of `symfony/psr-http-message-bridge` to allow installing it on a project that uses Symfony `3.4.x` components only (#480)
 

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -15,6 +15,11 @@ parameters:
 			count: 1
 			path: src/EventListener/ErrorListener.php
 
+		-
+			message: "#^Method Sentry\\\\SentryBundle\\\\EventListener\\\\TracingRequestListener\\:\\:getHttpFlavor\\(\\) never returns null so it can be removed from the return typehint\\.$#"
+			count: 1
+			path: src/EventListener/TracingRequestListener.php
+
 		-
 			message: "#^Call to an undefined method Doctrine\\\\DBAL\\\\Driver\\|Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\Compatibility\\\\ExceptionConverterDriverInterface\\:\\:connect\\(\\)\\.$#"
 			count: 1

diff --git a/src/EventListener/TracingRequestListener.php b/src/EventListener/TracingRequestListener.php
@@ -68,14 +68,18 @@ public function handleKernelTerminateEvent(RequestListenerTerminateEvent $event)
     private function getTags(Request $request): array
     {
         $client = $this->hub->getClient();
+        $httpFlavor = $this->getHttpFlavor($request);
         $tags = [
             'net.host.port' => (string) $request->getPort(),
             'http.method' => $request->getMethod(),
             'http.url' => $request->getUri(),
-            'http.flavor' => $this->getHttpFlavor($request),
             'route' => $this->getRouteName($request),
         ];
 
+        if (null !== $httpFlavor) {
+            $tags['http.flavor'] = $httpFlavor;
+        }
+
         if (false !== filter_var($request->getHost(), \FILTER_VALIDATE_IP)) {
             $tags['net.host.ip'] = $request->getHost();
         } else {
@@ -94,11 +98,11 @@ private function getTags(Request $request): array
      *
      * @param Request $request The HTTP request
      */
-    protected function getHttpFlavor(Request $request): string
+    private function getHttpFlavor(Request $request): ?string
     {
         $protocolVersion = $request->getProtocolVersion();
 
-        if (str_starts_with($protocolVersion, 'HTTP/')) {
+        if (null !== $protocolVersion && str_starts_with($protocolVersion, 'HTTP/')) {
             return substr($protocolVersion, \strlen('HTTP/'));
         }
 

diff --git a/tests/EventListener/TracingRequestListenerTest.php b/tests/EventListener/TracingRequestListenerTest.php
@@ -52,7 +52,7 @@ public function testHandleKernelRequestEvent(Options $options, Request $request,
         $transaction = new Transaction(new TransactionContext());
 
         $client = $this->createMock(ClientInterface::class);
-        $client->expects($this->once())
+        $client->expects($this->any())
             ->method('getOptions')
             ->willReturn($options);
 
@@ -63,7 +63,7 @@ public function testHandleKernelRequestEvent(Options $options, Request $request,
         $this->hub->expects($this->once())
             ->method('startTransaction')
             ->with($this->callback(function (TransactionContext $context) use ($expectedTransactionContext): bool {
-                $this->assertEquals($context, $expectedTransactionContext);
+                $this->assertEquals($expectedTransactionContext, $context);
 
                 return true;
             }))
@@ -326,6 +326,27 @@ public function handleKernelRequestEventDataProvider(): \Generator
             $request,
             $transactionContext,
         ];
+
+        $request = Request::createFromGlobals();
+        $request->server->set('REQUEST_TIME_FLOAT', 1613493597.010275);
+
+        $transactionContext = new TransactionContext();
+        $transactionContext->setName('GET http://:/');
+        $transactionContext->setOp('http.server');
+        $transactionContext->setStartTimestamp(1613493597.010275);
+        $transactionContext->setTags([
+            'net.host.port' => '',
+            'http.method' => 'GET',
+            'http.url' => 'http://:/',
+            'route' => '<unknown>',
+            'net.host.name' => '',
+        ]);
+
+        yield 'request.server.SERVER_PROTOCOL NOT EXISTS' => [
+            new Options(),
+            $request,
+            $transactionContext,
+        ];
     }
 
     public function testHandleKernelRequestEventDoesNothingIfRequestTypeIsSubRequest(): void