From c016dc2496833c73747a5e5babf04c41791a5118 Mon Sep 17 00:00:00 2001 From: Stefano Arlandini Date: Wed, 5 May 2021 09:44:53 +0200 Subject: [PATCH] Fix fatal error when the `SERVER_PROTOCOL` header is missing (#495) --- CHANGELOG.md | 3 ++- phpstan-baseline.neon | 5 ++++ src/EventListener/TracingRequestListener.php | 10 +++++--- .../TracingRequestListenerTest.php | 25 +++++++++++++++++-- 4 files changed, 37 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa5404a3..828c860d 100644 --- 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 index 13171eb5..5f89c5d7 100644 --- 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 index eb3dcf82..760a6bad 100644 --- 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 index 579c69b8..17c8c793 100644 --- 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' => '', + 'net.host.name' => '', + ]); + + yield 'request.server.SERVER_PROTOCOL NOT EXISTS' => [ + new Options(), + $request, + $transactionContext, + ]; } public function testHandleKernelRequestEventDoesNothingIfRequestTypeIsSubRequest(): void