getsentry / sentry-symfony

The official Symfony SDK for Sentry (sentry.io)
https://sentry.io
MIT License
695 stars 170 forks source link

Undeclared dependency in `AbstractTraceableHttpClient` #714

Closed Dormilich closed 1 year ago

Dormilich commented 1 year ago

Environment

PHP: 7.4.33 sentry/sdk: (omitted) sentry/sentry: 3.17.0 sentry/sentry-symfony: 4.7.0 symfony/*: 5.4.21

Steps to Reproduce

Note: Installation done through composer

  1. install Symfony LTS (5.4)
  2. install symfony/http-client and nyholm/psr7
  3. disable sentry/sdk in composer.json
  4. install sentry/sentry-symfony
  5. use default configuration (tracing enabled)
  6. trigger the error by running code that makes an HTTP request

Expected Result

The HTTP request should be processed without error.

Actual Result

The requests throws a class-not-found error.

NOTICE: PHP message: php.CRITICAL: Uncaught Error: Class 'GuzzleHttp\Psr7\Uri' not found {"exception":"[object] (Error(code: 0): Class 'GuzzleHttp\\Psr7\\Uri' not found at /app/vendor/sentry/sentry-symfony/src/Tracing/HttpClient/AbstractTraceableHttpClient.php:54)"} []

The reason for this error is that GuzzleHttp\Psr7\Uri is used in AbstractTraceableHttpClient::request() without declaring it a dependency in either sentry/sentry-symfony or sentry/sentry. However, guzzlehttp/psr7 is a dependency of sentry/sdk.

A possible fix were to have a dependency on Psr\Http\Message\UriFactoryInterface in AbstractTraceableHttpClient since that is already required by sentry/sentry.

Jean85 commented 1 year ago

Is this a duplicate of #707?

cleptric commented 1 year ago

I'll prepare a release that includes the fix for this issue (https://github.com/getsentry/sentry-symfony/pull/708). Sorry about that!

Dormilich commented 1 year ago

Meh, didn't think of looking up closed issues, sorry.

Dormilich commented 1 year ago

I would prefer if this were solved without a direct dependency on guzzlehttp/psr7 as the affected code does not require a specific implementation to achieve its goal ($partialUri could be created from any UriInterface instance).

Jean85 commented 1 year ago

@cleptric we should avoid similar issues in the future by adding https://packagist.org/packages/maglnet/composer-require-checker in CI

Dormilich commented 1 year ago

Btw, sentry/sentry has the same issue in GuzzleTracingMiddleware.

cleptric commented 1 year ago

Btw, sentry/sentry has the same issue in GuzzleTracingMiddleware.

No, see https://github.com/getsentry/sentry-php/issues/1521#issuecomment-1525909913

cleptric commented 1 year ago

Fixed in 4.8.0