From 54b6ad33c2568c0a6aa90e696be3d2dd8c5ee015 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Fri, 30 Sep 2022 14:04:34 +0200 Subject: [PATCH 1/4] Remove unused constructor argument --- src/Sentry/Laravel/ServiceProvider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sentry/Laravel/ServiceProvider.php b/src/Sentry/Laravel/ServiceProvider.php index 0b6bb9ff..596c4cac 100644 --- a/src/Sentry/Laravel/ServiceProvider.php +++ b/src/Sentry/Laravel/ServiceProvider.php @@ -206,7 +206,7 @@ protected function configureAndRegisterClient(): void }); $integrations[] = new SdkIntegration\RequestIntegration( - new LaravelRequestFetcher($app) + new LaravelRequestFetcher ); } From b5d88ba899f24f53e07d46072571142399b36176 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Fri, 30 Sep 2022 14:44:30 +0200 Subject: [PATCH 2/4] Backport PSR-7 request building to prevent errors --- composer.json | 3 -- .../Laravel/Http/LaravelRequestFetcher.php | 3 ++ .../Laravel/Http/SetRequestMiddleware.php | 48 ++++++++++++------- 3 files changed, 34 insertions(+), 20 deletions(-) diff --git a/composer.json b/composer.json index 816cd017..940dd906 100644 --- a/composer.json +++ b/composer.json @@ -33,9 +33,6 @@ "Sentry\\Laravel\\": "src/" } }, - "suggest": { - "zendframework/zend-diactoros": "When using Laravel >=5.1 - <=6.9 this package can help get more accurate request info, not used on Laravel >=6.10 anymore (https://laravel.com/docs/5.8/requests#psr7-requests)" - }, "require-dev": { "phpunit/phpunit": "^5.7 | ^6.5 | ^7.5 | ^8.4 | ^9.3", "laravel/framework": "5.0 - 5.8 | ^6.0 | ^7.0 | ^8.0 | ^9.0", diff --git a/src/Sentry/Laravel/Http/LaravelRequestFetcher.php b/src/Sentry/Laravel/Http/LaravelRequestFetcher.php index 8a652c52..f8abf0b0 100644 --- a/src/Sentry/Laravel/Http/LaravelRequestFetcher.php +++ b/src/Sentry/Laravel/Http/LaravelRequestFetcher.php @@ -18,6 +18,9 @@ public function fetchRequest(): ?ServerRequestInterface { $container = Container::getInstance(); + // If there is no request bound to the container + // we are not dealing with a HTTP request and there + // is no request to fetch for us so we can exit early. if (!$container->bound('request')) { return null; } diff --git a/src/Sentry/Laravel/Http/SetRequestMiddleware.php b/src/Sentry/Laravel/Http/SetRequestMiddleware.php index d06cbfff..ee788615 100644 --- a/src/Sentry/Laravel/Http/SetRequestMiddleware.php +++ b/src/Sentry/Laravel/Http/SetRequestMiddleware.php @@ -4,10 +4,11 @@ use Closure; use Illuminate\Container\Container; -use Illuminate\Contracts\Container\BindingResolutionException; use Illuminate\Http\Request; +use Nyholm\Psr7\Factory\Psr17Factory; use Psr\Http\Message\ServerRequestInterface; use Sentry\State\HubInterface; +use Symfony\Bridge\PsrHttpMessage\Factory\PsrHttpFactory; /** * This middleware caches a PSR-7 version of the request as early as possible. @@ -15,30 +16,43 @@ */ class SetRequestMiddleware { - /** - * Handle an incoming request. - * - * @param \Illuminate\Http\Request $request - * @param \Closure $next - * - * @return mixed - */ public function handle(Request $request, Closure $next) { $container = Container::getInstance(); if ($container->bound(HubInterface::class)) { - try { - $container->instance( - LaravelRequestFetcher::CONTAINER_PSR7_INSTANCE_KEY, - $container->make(ServerRequestInterface::class) - ); - } catch (BindingResolutionException $e) { - // Ignore problems getting the PSR-7 server request instance here - // In the Laravel request fetcher we have other fallbacks for that + $psrRequest = $this->resolvePsrRequest($request); + + if ($psrRequest !== null) { + $container->instance(LaravelRequestFetcher::CONTAINER_PSR7_INSTANCE_KEY, $psrRequest); } } return $next($request); } + + /** + * This code was copied from the Laravel codebase which was introduced in Laravel 6.x. + * + * The reason we have it copied here is because older (<6.x) versions of Laravel use a different + * method to construct the PSR-7 request object which requires other packages to create that object + * but most importantly it does not function when those packages are not available resulting in errors + * + * So long story short, this is here to backport functionality to Laravel <6.x + * if we drop support for those versions in the future we can reconsider this and + * move back to using the container binding provided by Laravel for the PSR-7 object + * + * @see https://github.com/laravel/framework/blob/cb550b5bdc2b2c4cf077082adabde0144a72d190/src/Illuminate/Routing/RoutingServiceProvider.php#L127-L146 + */ + private function resolvePsrRequest(Request $request): ?ServerRequestInterface + { + if (class_exists(Psr17Factory::class) && class_exists(PsrHttpFactory::class)) { + $psr17Factory = new Psr17Factory; + + return (new PsrHttpFactory($psr17Factory, $psr17Factory, $psr17Factory, $psr17Factory)) + ->createRequest($request); + } + + return null; + } } From b28ee5da20d794cde232e3ed32304effd54e8568 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Fri, 30 Sep 2022 14:49:04 +0200 Subject: [PATCH 3/4] Update comments --- src/Sentry/Laravel/Http/SetRequestMiddleware.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Sentry/Laravel/Http/SetRequestMiddleware.php b/src/Sentry/Laravel/Http/SetRequestMiddleware.php index ee788615..bbdfca4e 100644 --- a/src/Sentry/Laravel/Http/SetRequestMiddleware.php +++ b/src/Sentry/Laravel/Http/SetRequestMiddleware.php @@ -32,13 +32,13 @@ public function handle(Request $request, Closure $next) } /** - * This code was copied from the Laravel codebase which was introduced in Laravel 6.x. + * This code was copied from the Laravel codebase which was introduced in Laravel 6. * - * The reason we have it copied here is because older (<6.x) versions of Laravel use a different + * The reason we have it copied here is because older (<6.0) versions of Laravel use a different * method to construct the PSR-7 request object which requires other packages to create that object * but most importantly it does not function when those packages are not available resulting in errors * - * So long story short, this is here to backport functionality to Laravel <6.x + * So long story short, this is here to backport functionality to Laravel <6.0 * if we drop support for those versions in the future we can reconsider this and * move back to using the container binding provided by Laravel for the PSR-7 object * From 917f4711c7c9647e288b9608ea0bcfc05a1c9d8c Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Fri, 30 Sep 2022 14:49:17 +0200 Subject: [PATCH 4/4] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cb14e79..31af3d53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Fix not listening to queue events because `QueueManager` is registered as `queue` in the container and not by it's class name (#568) - Fix status code not populated on transaction if response did not inherit from `Illuminate\Http\Response` like `Illuminate\Http\JsonResponse` (#573) +- Fix broken `SetRequestMiddleware` on Laravel < 6.0 (#575) ## 2.13.0