From fa5145d9d7f0531291769b2296272ca4c57cf412 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Wed, 13 Sep 2023 10:54:20 +0200 Subject: [PATCH 01/20] Add a default cURL HTTP client Fix PHP 7.2 - 7.3 tests Fix CS Remove .relay folder --- composer.json | 11 +- phpstan-baseline.neon | 60 ++--- src/Client.php | 14 +- src/ClientBuilder.php | 78 +++--- src/ClientBuilderInterface.php | 10 - src/ClientInterface.php | 4 +- .../Authentication/SentryAuthentication.php | 78 ------ src/HttpClient/HttpClient.php | 158 ++++++++++++ src/HttpClient/HttpClientFactory.php | 165 ------------ src/HttpClient/HttpClientFactoryInterface.php | 22 -- src/HttpClient/HttpClientInterface.php | 10 + src/HttpClient/Plugin/GzipEncoderPlugin.php | 64 ----- src/HttpClient/Response.php | 63 +++++ src/Options.php | 44 ++++ src/Transport/DefaultTransportFactory.php | 74 ------ src/Transport/HttpTransport.php | 99 ++----- src/Transport/NullTransport.php | 17 +- src/Transport/RateLimiter.php | 12 +- src/{Response.php => Transport/Result.php} | 12 +- .../ResultStatus.php} | 4 +- src/Transport/TransportFactoryInterface.php | 21 -- src/Transport/TransportInterface.php | 24 +- tests/ClientBuilderTest.php | 32 --- tests/ClientTest.php | 84 +++--- .../SentryAuthenticationTest.php | 61 ----- tests/HttpClient/HttpClientFactoryTest.php | 104 -------- .../Plugin/GzipEncoderPluginTest.php | 40 --- .../Transport/DefaultTransportFactoryTest.php | 50 ---- tests/Transport/HttpTransportTest.php | 244 +++--------------- tests/Transport/NullTransportTest.php | 16 +- tests/Transport/RateLimiterTest.php | 41 ++- .../ResultStatusTest.php} | 46 ++-- ...errors_not_silencable_on_php_8_and_up.phpt | 30 +-- .../error_handler_captures_fatal_error.phpt | 34 +-- ...spects_capture_silenced_errors_option.phpt | 34 +-- ...espects_current_error_reporting_level.phpt | 30 +-- ..._option_regardless_of_error_reporting.phpt | 32 +-- ...tegration_respects_error_types_option.phpt | 28 +- ...rror_integration_captures_fatal_error.phpt | 28 +- ...tegration_respects_error_types_option.phpt | 28 +- 40 files changed, 590 insertions(+), 1416 deletions(-) delete mode 100644 src/HttpClient/Authentication/SentryAuthentication.php create mode 100644 src/HttpClient/HttpClient.php delete mode 100644 src/HttpClient/HttpClientFactory.php delete mode 100644 src/HttpClient/HttpClientFactoryInterface.php create mode 100644 src/HttpClient/HttpClientInterface.php delete mode 100644 src/HttpClient/Plugin/GzipEncoderPlugin.php create mode 100644 src/HttpClient/Response.php delete mode 100644 src/Transport/DefaultTransportFactory.php rename src/{Response.php => Transport/Result.php} (75%) rename src/{ResponseStatus.php => Transport/ResultStatus.php} (97%) delete mode 100644 src/Transport/TransportFactoryInterface.php delete mode 100644 tests/HttpClient/Authentication/SentryAuthenticationTest.php delete mode 100644 tests/HttpClient/HttpClientFactoryTest.php delete mode 100644 tests/HttpClient/Plugin/GzipEncoderPluginTest.php delete mode 100644 tests/Transport/DefaultTransportFactoryTest.php rename tests/{ResponseStatusTest.php => Transport/ResultStatusTest.php} (53%) diff --git a/composer.json b/composer.json index 32ea2f592..e882414c9 100644 --- a/composer.json +++ b/composer.json @@ -23,22 +23,15 @@ "php": "^7.2|^8.0", "ext-json": "*", "ext-mbstring": "*", - "guzzlehttp/promises": "^1.5.3|^2.0", + "ext-curl": "*", "jean85/pretty-package-versions": "^1.5|^2.0.4", - "php-http/async-client-implementation": "^1.0", - "php-http/client-common": "^1.5|^2.0", - "php-http/discovery": "^1.15", - "php-http/httplug": "^1.1|^2.0", - "php-http/message": "^1.5", - "php-http/message-factory": "^1.1", - "psr/http-factory": "^1.0", - "psr/http-factory-implementation": "^1.0", "psr/log": "^1.0|^2.0|^3.0", "symfony/options-resolver": "^3.4.43|^4.4.30|^5.0.11|^6.0", "symfony/polyfill-php80": "^1.17" }, "require-dev": { "friendsofphp/php-cs-fixer": "^2.19|3.4.*", + "guzzlehttp/promises": "^1.0|^2.0", "guzzlehttp/psr7": "^1.8.4|^2.1.1", "http-interop/http-factory-guzzle": "^1.0", "monolog/monolog": "^1.6|^2.0|^3.0", diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 39aa31fc7..124b73ebc 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -35,51 +35,6 @@ parameters: count: 1 path: src/Dsn.php - - - message: "#^Access to constant CONNECT_TIMEOUT on an unknown class GuzzleHttp\\\\RequestOptions\\.$#" - count: 1 - path: src/HttpClient/HttpClientFactory.php - - - - message: "#^Access to constant PROXY on an unknown class GuzzleHttp\\\\RequestOptions\\.$#" - count: 1 - path: src/HttpClient/HttpClientFactory.php - - - - message: "#^Access to constant TIMEOUT on an unknown class GuzzleHttp\\\\RequestOptions\\.$#" - count: 1 - path: src/HttpClient/HttpClientFactory.php - - - - message: "#^Call to static method create\\(\\) on an unknown class Symfony\\\\Component\\\\HttpClient\\\\HttpClient\\.$#" - count: 1 - path: src/HttpClient/HttpClientFactory.php - - - - message: "#^Call to static method createWithConfig\\(\\) on an unknown class Http\\\\Adapter\\\\Guzzle6\\\\Client\\.$#" - count: 1 - path: src/HttpClient/HttpClientFactory.php - - - - message: "#^Constructor of class Sentry\\\\HttpClient\\\\HttpClientFactory has an unused parameter \\$responseFactory\\.$#" - count: 1 - path: src/HttpClient/HttpClientFactory.php - - - - message: "#^Constructor of class Sentry\\\\HttpClient\\\\HttpClientFactory has an unused parameter \\$uriFactory\\.$#" - count: 1 - path: src/HttpClient/HttpClientFactory.php - - - - message: "#^Method Sentry\\\\HttpClient\\\\HttpClientFactory\\:\\:resolveClient\\(\\) should return Http\\\\Client\\\\HttpAsyncClient\\|Psr\\\\Http\\\\Client\\\\ClientInterface but returns Http\\\\Client\\\\Curl\\\\Client\\.$#" - count: 1 - path: src/HttpClient/HttpClientFactory.php - - - - message: "#^Method Sentry\\\\HttpClient\\\\HttpClientFactory\\:\\:resolveClient\\(\\) should return Http\\\\Client\\\\HttpAsyncClient\\|Psr\\\\Http\\\\Client\\\\ClientInterface but returns Symfony\\\\Component\\\\HttpClient\\\\HttplugClient\\.$#" - count: 1 - path: src/HttpClient/HttpClientFactory.php - - message: "#^Property Sentry\\\\Integration\\\\IgnoreErrorsIntegration\\:\\:\\$options \\(array\\{ignore_exceptions\\: array\\\\>, ignore_tags\\: array\\\\}\\) does not accept array\\.$#" count: 1 @@ -140,6 +95,11 @@ parameters: count: 1 path: src/Options.php + - + message: "#^Method Sentry\\\\Options\\:\\:getHttpClient\\(\\) should return Sentry\\\\HttpClient\\\\HttpClientInterface\\|null but returns mixed\\.$#" + count: 1 + path: src/Options.php + - message: "#^Method Sentry\\\\Options\\:\\:getHttpConnectTimeout\\(\\) should return float but returns mixed\\.$#" count: 1 @@ -150,6 +110,11 @@ parameters: count: 1 path: src/Options.php + - + message: "#^Method Sentry\\\\Options\\:\\:getHttpProxyAuthentication\\(\\) should return string\\|null but returns mixed\\.$#" + count: 1 + path: src/Options.php + - message: "#^Method Sentry\\\\Options\\:\\:getHttpTimeout\\(\\) should return float but returns mixed\\.$#" count: 1 @@ -245,6 +210,11 @@ parameters: count: 1 path: src/Options.php + - + message: "#^Method Sentry\\\\Options\\:\\:getTransport\\(\\) should return Sentry\\\\Transport\\\\TransportInterface\\|null but returns mixed\\.$#" + count: 1 + path: src/Options.php + - message: "#^Method Sentry\\\\Options\\:\\:hasDefaultIntegrations\\(\\) should return bool but returns mixed\\.$#" count: 1 diff --git a/src/Client.php b/src/Client.php index 3d0b6d305..33d74927d 100644 --- a/src/Client.php +++ b/src/Client.php @@ -4,7 +4,6 @@ namespace Sentry; -use GuzzleHttp\Promise\PromiseInterface; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use Sentry\Integration\IntegrationInterface; @@ -13,6 +12,7 @@ use Sentry\Serializer\RepresentationSerializerInterface; use Sentry\Serializer\SerializerInterface; use Sentry\State\Scope; +use Sentry\Transport\Result; use Sentry\Transport\TransportInterface; /** @@ -173,14 +173,18 @@ public function captureEvent(Event $event, ?EventHint $hint = null, ?Scope $scop } try { - /** @var Response $response */ - $response = $this->transport->send($event)->wait(); - $event = $response->getEvent(); + /** @var Result $result */ + $result = $this->transport->send($event); + $event = $result->getEvent(); if (null !== $event) { return $event->getId(); } } catch (\Throwable $exception) { + $this->logger->error( + sprintf('Failed to send the event to Sentry. Reason: "%s".', $exception->getMessage()), + ['exception' => $exception, 'event' => $event] + ); } return null; @@ -216,7 +220,7 @@ public function getIntegration(string $className): ?IntegrationInterface /** * {@inheritdoc} */ - public function flush(?int $timeout = null): PromiseInterface + public function flush(?int $timeout = null): Result { return $this->transport->close($timeout); } diff --git a/src/ClientBuilder.php b/src/ClientBuilder.php index 8cdcc7317..3fffadbee 100644 --- a/src/ClientBuilder.php +++ b/src/ClientBuilder.php @@ -4,13 +4,13 @@ namespace Sentry; -use Http\Discovery\Psr17FactoryDiscovery; use Psr\Log\LoggerInterface; -use Sentry\HttpClient\HttpClientFactory; +use Sentry\HttpClient\HttpClient; +use Sentry\HttpClient\HttpClientInterface; +use Sentry\Serializer\PayloadSerializer; use Sentry\Serializer\RepresentationSerializerInterface; use Sentry\Serializer\SerializerInterface; -use Sentry\Transport\DefaultTransportFactory; -use Sentry\Transport\TransportFactoryInterface; +use Sentry\Transport\HttpTransport; use Sentry\Transport\TransportInterface; /** @@ -26,14 +26,14 @@ final class ClientBuilder implements ClientBuilderInterface private $options; /** - * @var TransportFactoryInterface|null The transport factory + * @var TransportInterface The transport */ - private $transportFactory; + private $transport; /** - * @var TransportInterface|null The transport + * @var HttpClientInterface The HTTP client */ - private $transport; + private $httpClient; /** * @var SerializerInterface|null The serializer to be injected in the client @@ -68,6 +68,13 @@ final class ClientBuilder implements ClientBuilderInterface public function __construct(Options $options = null) { $this->options = $options ?? new Options(); + + $this->httpClient = $this->options->getHttpClient() ?? new HttpClient($this->options, $this->sdkIdentifier, $this->sdkVersion); + $this->transport = $this->options->getTransport() ?? new HttpTransport( + $this->httpClient, + new PayloadSerializer($this->options), + $this->logger + ); } /** @@ -136,60 +143,35 @@ public function setSdkVersion(string $sdkVersion): ClientBuilderInterface return $this; } - /** - * {@inheritdoc} - */ - public function setTransportFactory(TransportFactoryInterface $transportFactory): ClientBuilderInterface + public function getTransport(): TransportInterface { - $this->transportFactory = $transportFactory; - - return $this; + return $this->transport; } - /** - * {@inheritdoc} - */ - public function getClient(): ClientInterface + public function setTransport(TransportInterface $transport): ClientBuilderInterface { - $this->transport = $this->transport ?? $this->createTransportInstance(); + $this->transport = $transport; - return new Client($this->options, $this->transport, $this->sdkIdentifier, $this->sdkVersion, $this->serializer, $this->representationSerializer, $this->logger); + return $this; } - /** - * Creates a new instance of the transport mechanism. - */ - private function createTransportInstance(): TransportInterface + public function getHttpClient(): HttpClientInterface { - if (null !== $this->transport) { - return $this->transport; - } + return $this->httpClient; + } - $transportFactory = $this->transportFactory ?? $this->createDefaultTransportFactory(); + public function setHttpClient(HttpClientInterface $httpClient): ClientBuilderInterface + { + $this->httpClient = $httpClient; - return $transportFactory->create($this->options); + return $this; } /** - * Creates a new instance of the {@see DefaultTransportFactory} factory. + * {@inheritdoc} */ - private function createDefaultTransportFactory(): DefaultTransportFactory + public function getClient(): ClientInterface { - $streamFactory = Psr17FactoryDiscovery::findStreamFactory(); - $httpClientFactory = new HttpClientFactory( - null, - null, - $streamFactory, - null, - $this->sdkIdentifier, - $this->sdkVersion - ); - - return new DefaultTransportFactory( - $streamFactory, - Psr17FactoryDiscovery::findRequestFactory(), - $httpClientFactory, - $this->logger - ); + return new Client($this->options, $this->transport, $this->sdkIdentifier, $this->sdkVersion, $this->serializer, $this->representationSerializer, $this->logger); } } diff --git a/src/ClientBuilderInterface.php b/src/ClientBuilderInterface.php index 35d67b12a..8d3fae1b6 100644 --- a/src/ClientBuilderInterface.php +++ b/src/ClientBuilderInterface.php @@ -7,7 +7,6 @@ use Psr\Log\LoggerInterface; use Sentry\Serializer\RepresentationSerializerInterface; use Sentry\Serializer\SerializerInterface; -use Sentry\Transport\TransportFactoryInterface; /** * A configurable builder for Client objects. @@ -64,15 +63,6 @@ public function setRepresentationSerializer(RepresentationSerializerInterface $r */ public function setLogger(LoggerInterface $logger): ClientBuilderInterface; - /** - * Sets the transport factory. - * - * @param TransportFactoryInterface $transportFactory The transport factory - * - * @return $this - */ - public function setTransportFactory(TransportFactoryInterface $transportFactory): ClientBuilderInterface; - /** * Sets the SDK identifier to be passed onto {@see Event} and HTTP User-Agent header. * diff --git a/src/ClientInterface.php b/src/ClientInterface.php index 3f6d06f38..b8774385d 100644 --- a/src/ClientInterface.php +++ b/src/ClientInterface.php @@ -4,9 +4,9 @@ namespace Sentry; -use GuzzleHttp\Promise\PromiseInterface; use Sentry\Integration\IntegrationInterface; use Sentry\State\Scope; +use Sentry\Transport\Result; /** * This interface must be implemented by all Raven client classes. @@ -78,5 +78,5 @@ public function getIntegration(string $className): ?IntegrationInterface; * * @param int|null $timeout Maximum time in seconds the client should wait */ - public function flush(?int $timeout = null): PromiseInterface; + public function flush(?int $timeout = null): Result; } diff --git a/src/HttpClient/Authentication/SentryAuthentication.php b/src/HttpClient/Authentication/SentryAuthentication.php deleted file mode 100644 index 917487e83..000000000 --- a/src/HttpClient/Authentication/SentryAuthentication.php +++ /dev/null @@ -1,78 +0,0 @@ - - */ -final class SentryAuthentication implements AuthenticationInterface -{ - /** - * @var Options The Sentry client configuration - */ - private $options; - - /** - * @var string The SDK identifier - */ - private $sdkIdentifier; - - /** - * @var string The SDK version - */ - private $sdkVersion; - - /** - * Constructor. - * - * @param Options $options The Sentry client configuration - * @param string $sdkIdentifier The Sentry SDK identifier in use - * @param string $sdkVersion The Sentry SDK version in use - */ - public function __construct(Options $options, string $sdkIdentifier, string $sdkVersion) - { - $this->options = $options; - $this->sdkIdentifier = $sdkIdentifier; - $this->sdkVersion = $sdkVersion; - } - - /** - * {@inheritdoc} - */ - public function authenticate(RequestInterface $request): RequestInterface - { - $dsn = $this->options->getDsn(); - - if (null === $dsn) { - return $request; - } - - $data = [ - 'sentry_version' => Client::PROTOCOL_VERSION, - 'sentry_client' => $this->sdkIdentifier . '/' . $this->sdkVersion, - 'sentry_key' => $dsn->getPublicKey(), - ]; - - if (null !== $dsn->getSecretKey()) { - $data['sentry_secret'] = $dsn->getSecretKey(); - } - - $headers = []; - - foreach ($data as $headerKey => $headerValue) { - $headers[] = $headerKey . '=' . $headerValue; - } - - return $request->withHeader('X-Sentry-Auth', 'Sentry ' . implode(', ', $headers)); - } -} diff --git a/src/HttpClient/HttpClient.php b/src/HttpClient/HttpClient.php new file mode 100644 index 000000000..e2df6598e --- /dev/null +++ b/src/HttpClient/HttpClient.php @@ -0,0 +1,158 @@ +options = $options; + $this->sdkIdentifier = $sdkIdentifier; + $this->sdkVersion = $sdkVersion; + } + + public function sendRequest(string $requestData): Response + { + $dsn = $this->options->getDsn(); + if (null === $dsn) { + throw new \RuntimeException('The DSN option must be set to use the HttpClient.'); + } + + $curlHandle = curl_init(); + curl_setopt($curlHandle, \CURLOPT_URL, $dsn->getEnvelopeApiEndpointUrl()); + curl_setopt($curlHandle, \CURLOPT_HTTPHEADER, $this->getRequestHeaders()); + curl_setopt($curlHandle, \CURLOPT_USERAGENT, $this->sdkIdentifier . '/' . $this->sdkVersion); + curl_setopt($curlHandle, \CURLOPT_TIMEOUT, $this->options->getHttpTimeout()); + curl_setopt($curlHandle, \CURLOPT_CONNECTTIMEOUT, $this->options->getHttpConnectTimeout()); + curl_setopt($curlHandle, \CURLOPT_SSL_VERIFYPEER, true); + curl_setopt($curlHandle, \CURLOPT_ENCODING, ''); + curl_setopt($curlHandle, \CURLOPT_POST, true); + curl_setopt($curlHandle, \CURLOPT_POSTFIELDS, $requestData); + curl_setopt($curlHandle, \CURLOPT_RETURNTRANSFER, true); + curl_setopt($curlHandle, \CURLOPT_HEADER, true); + /** + * @TODO(michi) make this configurable + */ + curl_setopt($curlHandle, \CURLOPT_SSL_VERIFYPEER, true); + /** + * @TODO(michi) make this configurable + * + * If we add support for CURL_HTTP_VERSION_2_0, we need + * case-insensitive header handling, as HTTP 2.0 headers + * are all lowercase. + */ + curl_setopt($curlHandle, \CURLOPT_HTTP_VERSION, \CURL_HTTP_VERSION_1_1); + + $httpProxy = $this->options->getHttpProxy(); + if (null !== $httpProxy) { + curl_setopt($curlHandle, \CURLOPT_PROXY, $httpProxy); + } + + $httpProxyAuthentication = $this->options->getHttpProxyAuthentication(); + if (null !== $httpProxyAuthentication) { + curl_setopt($curlHandle, \CURLOPT_PROXYUSERPWD, $httpProxyAuthentication); + } + + /** + * @TODO(michi) add request compression (gzip/brotli) depending on availiable extensions + */ + $body = curl_exec($curlHandle); + + if (false === $body) { + $errorCode = curl_errno($curlHandle); + $error = curl_error($curlHandle); + curl_close($curlHandle); + + $message = 'cURL Error (' . $errorCode . ') ' . $error; + + return new Response(0, [], $message); + } + + $statusCode = curl_getinfo($curlHandle, \CURLINFO_HTTP_CODE); + $headerSize = curl_getinfo($curlHandle, \CURLINFO_HEADER_SIZE); + $headers = $this->getResponseHeaders($headerSize, (string) $body); + + curl_close($curlHandle); + + return new Response($statusCode, $headers, ''); + } + + /** + * @return string[] + */ + protected function getRequestHeaders(): array + { + $headers = [ + 'Content-Type' => 'application/x-sentry-envelope', + ]; + + $dsn = $this->options->getDsn(); + if (null === $dsn) { + return $headers; + } + + $data = [ + 'sentry_version' => Client::PROTOCOL_VERSION, + 'sentry_client' => $this->sdkIdentifier . '/' . $this->sdkVersion, + 'sentry_key' => $dsn->getPublicKey(), + ]; + + if (null !== $dsn->getSecretKey()) { + $data['sentry_secret'] = $dsn->getSecretKey(); + } + + $authHeader = []; + foreach ($data as $headerKey => $headerValue) { + $authHeader[] = $headerKey . '=' . $headerValue; + } + + return array_merge($headers, [ + 'X-Sentry-Auth' => 'Sentry ' . implode(', ', $authHeader), + ]); + } + + /** + * @TODO(michi) This might need a bit more love, + * but we only really care about X-Sentry-Rate-Limits and Retry-After + * + * @return string[] + */ + protected function getResponseHeaders(?int $headerSize, string $body): array + { + $headers = []; + $rawHeaders = explode("\r\n", trim(substr($body, 0, $headerSize))); + + foreach ($rawHeaders as $value) { + if (!str_contains($value, ':')) { + continue; + } + [$name, $value] = explode(':', $value, 2); + $value = trim($value); + $name = trim($name); + + $headers[$name] = $value; + } + + return $headers; + } +} diff --git a/src/HttpClient/HttpClientFactory.php b/src/HttpClient/HttpClientFactory.php deleted file mode 100644 index ba12123a4..000000000 --- a/src/HttpClient/HttpClientFactory.php +++ /dev/null @@ -1,165 +0,0 @@ -streamFactory = $streamFactory; - $this->httpClient = $httpClient; - $this->sdkIdentifier = $sdkIdentifier; - $this->sdkVersion = $sdkVersion; - } - - /** - * {@inheritdoc} - */ - public function create(Options $options): HttpAsyncClientInterface - { - if (null === $options->getDsn()) { - throw new \RuntimeException('Cannot create an HTTP client without the Sentry DSN set in the options.'); - } - - if (null !== $this->httpClient && null !== $options->getHttpProxy()) { - throw new \RuntimeException('The "http_proxy" option does not work together with a custom HTTP client.'); - } - - $httpClient = $this->httpClient ?? $this->resolveClient($options); - - $httpClientPlugins = [ - new HeaderSetPlugin(['User-Agent' => $this->sdkIdentifier . '/' . $this->sdkVersion]), - new AuthenticationPlugin(new SentryAuthentication($options, $this->sdkIdentifier, $this->sdkVersion)), - new RetryPlugin(['retries' => $options->getSendAttempts(false)]), - new ErrorPlugin(['only_server_exception' => true]), - ]; - - if ($options->isCompressionEnabled()) { - $httpClientPlugins[] = new GzipEncoderPlugin($this->streamFactory); - $httpClientPlugins[] = new DecoderPlugin(); - } - - return new PluginClient($httpClient, $httpClientPlugins); - } - - /** - * @return ClientInterface|HttpAsyncClientInterface - */ - private function resolveClient(Options $options) - { - if (class_exists(SymfonyHttplugClient::class)) { - $symfonyConfig = [ - 'timeout' => $options->getHttpConnectTimeout(), - 'max_duration' => $options->getHttpTimeout(), - 'http_version' => $options->isCompressionEnabled() ? '1.1' : null, - ]; - - if (null !== $options->getHttpProxy()) { - $symfonyConfig['proxy'] = $options->getHttpProxy(); - } - - return new SymfonyHttplugClient(SymfonyHttpClient::create($symfonyConfig)); - } - - if (class_exists(Guzzle7HttpClient::class) || class_exists(Guzzle6HttpClient::class)) { - $guzzleConfig = [ - GuzzleHttpClientOptions::TIMEOUT => $options->getHttpTimeout(), - GuzzleHttpClientOptions::CONNECT_TIMEOUT => $options->getHttpConnectTimeout(), - ]; - - if (null !== $options->getHttpProxy()) { - $guzzleConfig[GuzzleHttpClientOptions::PROXY] = $options->getHttpProxy(); - } - - if (class_exists(Guzzle7HttpClient::class)) { - return Guzzle7HttpClient::createWithConfig($guzzleConfig); - } - - return Guzzle6HttpClient::createWithConfig($guzzleConfig); - } - - if (class_exists(CurlHttpClient::class)) { - $curlConfig = [ - \CURLOPT_TIMEOUT => $options->getHttpTimeout(), - \CURLOPT_HTTP_VERSION => $options->isCompressionEnabled() ? \CURL_HTTP_VERSION_1_1 : \CURL_HTTP_VERSION_NONE, - \CURLOPT_CONNECTTIMEOUT => $options->getHttpConnectTimeout(), - ]; - - if (null !== $options->getHttpProxy()) { - $curlConfig[\CURLOPT_PROXY] = $options->getHttpProxy(); - } - - return new CurlHttpClient(null, null, $curlConfig); - } - - if (null !== $options->getHttpProxy()) { - throw new \RuntimeException('The "http_proxy" option requires either the "php-http/curl-client", the "symfony/http-client" or the "php-http/guzzle6-adapter" package to be installed.'); - } - - return HttpAsyncClientDiscovery::find(); - } -} diff --git a/src/HttpClient/HttpClientFactoryInterface.php b/src/HttpClient/HttpClientFactoryInterface.php deleted file mode 100644 index a0b409d70..000000000 --- a/src/HttpClient/HttpClientFactoryInterface.php +++ /dev/null @@ -1,22 +0,0 @@ - - */ -final class GzipEncoderPlugin implements PluginInterface -{ - /** - * @var StreamFactoryInterface The PSR-17 stream factory - */ - private $streamFactory; - - /** - * Constructor. - * - * @param StreamFactoryInterface $streamFactory The stream factory - * - * @throws \RuntimeException If the zlib extension is not enabled - */ - public function __construct(StreamFactoryInterface $streamFactory) - { - if (!\extension_loaded('zlib')) { - throw new \RuntimeException('The "zlib" extension must be enabled to use this plugin.'); - } - - $this->streamFactory = $streamFactory; - } - - /** - * {@inheritdoc} - */ - public function handleRequest(RequestInterface $request, callable $next, callable $first): PromiseInterface - { - $requestBody = $request->getBody(); - - if ($requestBody->isSeekable()) { - $requestBody->rewind(); - } - - // Instead of using a stream filter we have to compress the whole request - // body in one go to work around a PHP bug. See https://github.com/getsentry/sentry-php/pull/877 - $encodedBody = gzcompress($requestBody->getContents(), -1, \ZLIB_ENCODING_GZIP); - - if (false === $encodedBody) { - throw new \RuntimeException('Failed to GZIP-encode the request body.'); - } - - $request = $request->withHeader('Content-Encoding', 'gzip'); - $request = $request->withBody($this->streamFactory->createStream($encodedBody)); - - return $next($request); - } -} diff --git a/src/HttpClient/Response.php b/src/HttpClient/Response.php new file mode 100644 index 000000000..b6b8710c5 --- /dev/null +++ b/src/HttpClient/Response.php @@ -0,0 +1,63 @@ +statusCode = $statusCode; + $this->headers = $headers; + $this->error = $error; + } + + public function getStatusCode(): int + { + return $this->statusCode; + } + + public function isSuccess(): bool + { + return $this->statusCode >= 200 && $this->statusCode <= 299; + } + + public function hasHeader(string $headerName): bool + { + return \array_key_exists($headerName, $this->headers); + } + + public function getHeaderLine(string $headerName): string + { + return $this->headers[$headerName] ?? ''; + } + + public function getError(): string + { + return $this->error; + } + + public function hasError(): bool + { + return '' !== $this->error; + } +} diff --git a/src/Options.php b/src/Options.php index c78416252..a14578b83 100644 --- a/src/Options.php +++ b/src/Options.php @@ -4,8 +4,10 @@ namespace Sentry; +use Sentry\HttpClient\HttpClientInterface; use Sentry\Integration\ErrorListenerIntegration; use Sentry\Integration\IntegrationInterface; +use Sentry\Transport\TransportInterface; use Symfony\Component\OptionsResolver\Options as SymfonyOptions; use Symfony\Component\OptionsResolver\OptionsResolver; @@ -648,6 +650,30 @@ public function getIntegrations() return $this->options['integrations']; } + public function setTransport(TransportInterface $transport): void + { + $options = array_merge($this->options, ['transport' => $transport]); + + $this->options = $this->resolver->resolve($options); + } + + public function getTransport(): ?TransportInterface + { + return $this->options['transport']; + } + + public function setHttpClient(HttpClientInterface $httpClient): void + { + $options = array_merge($this->options, ['http_client' => $httpClient]); + + $this->options = $this->resolver->resolve($options); + } + + public function getHttpClient(): ?HttpClientInterface + { + return $this->options['http_client']; + } + /** * Should default PII be sent by default. */ @@ -728,6 +754,18 @@ public function setHttpProxy(?string $httpProxy): void $this->options = $this->resolver->resolve($options); } + public function getHttpProxyAuthentication(): ?string + { + return $this->options['http_proxy_authentication']; + } + + public function setHttpProxyAuthentication(?string $httpProxy): void + { + $options = array_merge($this->options, ['http_proxy_authentication' => $httpProxy]); + + $this->options = $this->resolver->resolve($options); + } + /** * Gets the maximum number of seconds to wait while trying to connect to a server. */ @@ -925,7 +963,10 @@ private function configureOptions(OptionsResolver $resolver): void 'in_app_include' => [], 'send_default_pii' => false, 'max_value_length' => 1024, + 'transport' => null, + 'http_client' => null, 'http_proxy' => null, + 'http_proxy_authentication' => null, 'http_connect_timeout' => self::DEFAULT_HTTP_CONNECT_TIMEOUT, 'http_timeout' => self::DEFAULT_HTTP_TIMEOUT, 'capture_silenced_errors' => false, @@ -963,7 +1004,10 @@ private function configureOptions(OptionsResolver $resolver): void $resolver->setAllowedTypes('send_default_pii', 'bool'); $resolver->setAllowedTypes('default_integrations', 'bool'); $resolver->setAllowedTypes('max_value_length', 'int'); + $resolver->setAllowedTypes('transport', ['null', 'Sentry\\Transport\\TransportInterface']); + $resolver->setAllowedTypes('http_client', ['null', 'Sentry\\HttpCLient\\HttpCLientInterface']); $resolver->setAllowedTypes('http_proxy', ['null', 'string']); + $resolver->setAllowedTypes('http_proxy_authentication', ['null', 'string']); $resolver->setAllowedTypes('http_connect_timeout', ['int', 'float']); $resolver->setAllowedTypes('http_timeout', ['int', 'float']); $resolver->setAllowedTypes('capture_silenced_errors', 'bool'); diff --git a/src/Transport/DefaultTransportFactory.php b/src/Transport/DefaultTransportFactory.php deleted file mode 100644 index 2d0c1f0b8..000000000 --- a/src/Transport/DefaultTransportFactory.php +++ /dev/null @@ -1,74 +0,0 @@ -streamFactory = $streamFactory; - $this->requestFactory = $requestFactory; - $this->httpClientFactory = $httpClientFactory; - $this->logger = $logger; - } - - /** - * {@inheritdoc} - */ - public function create(Options $options): TransportInterface - { - if (null === $options->getDsn()) { - return new NullTransport(); - } - - return new HttpTransport( - $options, - $this->httpClientFactory->create($options), - $this->streamFactory, - $this->requestFactory, - new PayloadSerializer($options), - $this->logger - ); - } -} diff --git a/src/Transport/HttpTransport.php b/src/Transport/HttpTransport.php index 9aa844edb..203efe32d 100644 --- a/src/Transport/HttpTransport.php +++ b/src/Transport/HttpTransport.php @@ -4,50 +4,22 @@ namespace Sentry\Transport; -use GuzzleHttp\Promise\FulfilledPromise; -use GuzzleHttp\Promise\PromiseInterface; -use GuzzleHttp\Promise\RejectedPromise; -use Http\Client\HttpAsyncClient as HttpAsyncClientInterface; -use Psr\Http\Message\RequestFactoryInterface; -use Psr\Http\Message\ResponseInterface; -use Psr\Http\Message\StreamFactoryInterface; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use Sentry\Event; -use Sentry\EventType; -use Sentry\Options; -use Sentry\Response; -use Sentry\ResponseStatus; +use Sentry\HttpClient\HttpClientInterface; use Sentry\Serializer\PayloadSerializerInterface; /** - * This transport sends the events using a syncronous HTTP client that will - * delay sending of the requests until the shutdown of the application. - * - * @author Stefano Arlandini + * @internal */ -final class HttpTransport implements TransportInterface +class HttpTransport implements TransportInterface { /** - * @var Options The Sentry client options - */ - private $options; - - /** - * @var HttpAsyncClientInterface The HTTP client + * @var HttpClientInterface The HTTP client */ private $httpClient; - /** - * @var StreamFactoryInterface The PSR-7 stream factory - */ - private $streamFactory; - - /** - * @var RequestFactoryInterface The PSR-7 request factory - */ - private $requestFactory; - /** * @var PayloadSerializerInterface The event serializer */ @@ -64,27 +36,16 @@ final class HttpTransport implements TransportInterface private $rateLimiter; /** - * Constructor. - * - * @param Options $options The Sentry client configuration - * @param HttpAsyncClientInterface $httpClient The HTTP client - * @param StreamFactoryInterface $streamFactory The PSR-7 stream factory - * @param RequestFactoryInterface $requestFactory The PSR-7 request factory + * @param HttpClientInterface $httpClient The HTTP client * @param PayloadSerializerInterface $payloadSerializer The event serializer * @param LoggerInterface|null $logger An instance of a PSR-3 logger */ public function __construct( - Options $options, - HttpAsyncClientInterface $httpClient, - StreamFactoryInterface $streamFactory, - RequestFactoryInterface $requestFactory, + HttpClientInterface $httpClient, PayloadSerializerInterface $payloadSerializer, ?LoggerInterface $logger = null ) { - $this->options = $options; $this->httpClient = $httpClient; - $this->streamFactory = $streamFactory; - $this->requestFactory = $requestFactory; $this->payloadSerializer = $payloadSerializer; $this->logger = $logger ?? new NullLogger(); $this->rateLimiter = new RateLimiter($this->logger); @@ -93,65 +54,49 @@ public function __construct( /** * {@inheritdoc} */ - public function send(Event $event): PromiseInterface + public function send(Event $event): Result { - $dsn = $this->options->getDsn(); - - if (null === $dsn) { - throw new \RuntimeException(sprintf('The DSN option must be set to use the "%s" transport.', self::class)); - } - $eventType = $event->getType(); - if ($this->rateLimiter->isRateLimited($eventType)) { $this->logger->warning( sprintf('Rate limit exceeded for sending requests of type "%s".', (string) $eventType), ['event' => $event] ); - return new RejectedPromise(new Response(ResponseStatus::rateLimit(), $event)); - } - - if ( - $this->options->isTracingEnabled() || - EventType::transaction() === $eventType || - EventType::checkIn() === $eventType - ) { - $request = $this->requestFactory->createRequest('POST', $dsn->getEnvelopeApiEndpointUrl()) - ->withHeader('Content-Type', 'application/x-sentry-envelope') - ->withBody($this->streamFactory->createStream($this->payloadSerializer->serialize($event))); - } else { - $request = $this->requestFactory->createRequest('POST', $dsn->getStoreApiEndpointUrl()) - ->withHeader('Content-Type', 'application/json') - ->withBody($this->streamFactory->createStream($this->payloadSerializer->serialize($event))); + return new Result(ResultStatus::rateLimit()); } try { - /** @var ResponseInterface $response */ - $response = $this->httpClient->sendAsyncRequest($request)->wait(); + $response = $this->httpClient->sendRequest($this->payloadSerializer->serialize($event)); } catch (\Throwable $exception) { $this->logger->error( sprintf('Failed to send the event to Sentry. Reason: "%s".', $exception->getMessage()), ['exception' => $exception, 'event' => $event] ); - return new RejectedPromise(new Response(ResponseStatus::failed(), $event)); + return new Result(ResultStatus::failed()); } - $sendResponse = $this->rateLimiter->handleResponse($event, $response); + $response = $this->rateLimiter->handleResponse($event, $response); + if ($response->isSuccess()) { + return new Result(ResultStatus::success(), $event); + } - if (ResponseStatus::success() === $sendResponse->getStatus()) { - return new FulfilledPromise($sendResponse); + if ($response->hasError()) { + $this->logger->error( + sprintf('Failed to send the event to Sentry. Reason: "%s".', $response->getError()), + ['event' => $event] + ); } - return new RejectedPromise($sendResponse); + return new Result(ResultStatus::createFromHttpStatusCode($response->getStatusCode())); } /** * {@inheritdoc} */ - public function close(?int $timeout = null): PromiseInterface + public function close(?int $timeout = null): Result { - return new FulfilledPromise(true); + return new Result(ResultStatus::success()); } } diff --git a/src/Transport/NullTransport.php b/src/Transport/NullTransport.php index 0c067e104..10d275565 100644 --- a/src/Transport/NullTransport.php +++ b/src/Transport/NullTransport.php @@ -4,32 +4,23 @@ namespace Sentry\Transport; -use GuzzleHttp\Promise\FulfilledPromise; -use GuzzleHttp\Promise\PromiseInterface; use Sentry\Event; -use Sentry\Response; -use Sentry\ResponseStatus; -/** - * This transport fakes the sending of events by just ignoring them. - * - * @author Stefano Arlandini - */ final class NullTransport implements TransportInterface { /** * {@inheritdoc} */ - public function send(Event $event): PromiseInterface + public function send(Event $event): Result { - return new FulfilledPromise(new Response(ResponseStatus::skipped(), $event)); + return new Result(ResultStatus::skipped(), $event); } /** * {@inheritdoc} */ - public function close(?int $timeout = null): PromiseInterface + public function close(?int $timeout = null): Result { - return new FulfilledPromise(true); + return new Result(ResultStatus::success()); } } diff --git a/src/Transport/RateLimiter.php b/src/Transport/RateLimiter.php index 4f75f234c..e6bcad986 100644 --- a/src/Transport/RateLimiter.php +++ b/src/Transport/RateLimiter.php @@ -4,13 +4,11 @@ namespace Sentry\Transport; -use Psr\Http\Message\ResponseInterface; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use Sentry\Event; use Sentry\EventType; -use Sentry\Response; -use Sentry\ResponseStatus; +use Sentry\HttpClient\Response; final class RateLimiter { @@ -47,10 +45,8 @@ public function __construct(?LoggerInterface $logger = null) $this->logger = $logger ?? new NullLogger(); } - public function handleResponse(Event $event, ResponseInterface $response): Response + public function handleResponse(Event $event, Response $response): Response { - $sendResponse = new Response(ResponseStatus::createFromHttpStatusCode($response->getStatusCode()), $event); - if ($this->handleRateLimit($response)) { $eventType = $event->getType(); $disabledUntil = $this->getDisabledUntil($eventType); @@ -61,7 +57,7 @@ public function handleResponse(Event $event, ResponseInterface $response): Respo ); } - return $sendResponse; + return $response; } public function isRateLimited(EventType $eventType): bool @@ -82,7 +78,7 @@ private function getDisabledUntil(EventType $eventType): int return max($this->rateLimits['all'] ?? 0, $this->rateLimits[$category] ?? 0); } - private function handleRateLimit(ResponseInterface $response): bool + private function handleRateLimit(Response $response): bool { $now = time(); diff --git a/src/Response.php b/src/Transport/Result.php similarity index 75% rename from src/Response.php rename to src/Transport/Result.php index 3a2d54173..7d5061bc4 100644 --- a/src/Response.php +++ b/src/Transport/Result.php @@ -2,16 +2,18 @@ declare(strict_types=1); -namespace Sentry; +namespace Sentry\Transport; + +use Sentry\Event; /** * This class contains the details of the sending operation of an event, e.g. * if it was sent successfully or if it was skipped because of some reason. */ -final class Response +class Result { /** - * @var ResponseStatus The status of the sending operation of the event + * @var ResultStatus The status of the sending operation of the event */ private $status; @@ -21,7 +23,7 @@ final class Response */ private $event; - public function __construct(ResponseStatus $status, ?Event $event = null) + public function __construct(ResultStatus $status, ?Event $event = null) { $this->status = $status; $this->event = $event; @@ -30,7 +32,7 @@ public function __construct(ResponseStatus $status, ?Event $event = null) /** * Gets the status of the sending operation of the event. */ - public function getStatus(): ResponseStatus + public function getStatus(): ResultStatus { return $this->status; } diff --git a/src/ResponseStatus.php b/src/Transport/ResultStatus.php similarity index 97% rename from src/ResponseStatus.php rename to src/Transport/ResultStatus.php index 2a2292a4f..e7df98ac0 100644 --- a/src/ResponseStatus.php +++ b/src/Transport/ResultStatus.php @@ -2,13 +2,13 @@ declare(strict_types=1); -namespace Sentry; +namespace Sentry\Transport; /** * This enum represents all possible reasons an event sending operation succeeded * or failed. */ -final class ResponseStatus implements \Stringable +class ResultStatus implements \Stringable { /** * @var string The value of the enum instance diff --git a/src/Transport/TransportFactoryInterface.php b/src/Transport/TransportFactoryInterface.php deleted file mode 100644 index e38f12b1f..000000000 --- a/src/Transport/TransportFactoryInterface.php +++ /dev/null @@ -1,21 +0,0 @@ - - */ interface TransportInterface { - /** - * Sends the given event. - * - * @param Event $event The event - * - * @return PromiseInterface Returns the ID of the event or `null` if it failed to be sent - */ - public function send(Event $event): PromiseInterface; + public function send(Event $event): Result; - /** - * Waits until all pending requests have been sent or the timeout expires. - * - * @param int|null $timeout Maximum time in seconds before the sending - * operation is interrupted - */ - public function close(?int $timeout = null): PromiseInterface; + public function close(?int $timeout = null): Result; } diff --git a/tests/ClientBuilderTest.php b/tests/ClientBuilderTest.php index 153986970..bd514bc24 100644 --- a/tests/ClientBuilderTest.php +++ b/tests/ClientBuilderTest.php @@ -10,9 +10,6 @@ use Sentry\Event; use Sentry\Integration\IntegrationInterface; use Sentry\Options; -use Sentry\Transport\HttpTransport; -use Sentry\Transport\NullTransport; -use Sentry\Transport\TransportInterface; final class ClientBuilderTest extends TestCase { @@ -24,24 +21,6 @@ public function testGetOptions() $this->assertSame($options, $clientBuilder->getOptions()); } - public function testHttpTransportIsUsedWhenServerIsConfigured(): void - { - $clientBuilder = ClientBuilder::create(['dsn' => 'http://public:secret@example.com/sentry/1']); - - $transport = $this->getTransport($clientBuilder->getClient()); - - $this->assertInstanceOf(HttpTransport::class, $transport); - } - - public function testNullTransportIsUsedWhenNoServerIsConfigured(): void - { - $clientBuilder = new ClientBuilder(); - - $transport = $this->getTransport($clientBuilder->getClient()); - - $this->assertInstanceOf(NullTransport::class, $transport); - } - public function testClientBuilderFallbacksToDefaultSdkIdentifierAndVersion(): void { $callbackCalled = false; @@ -91,17 +70,6 @@ public function testCreateWithNoOptionsIsTheSameAsDefaultOptions(): void ClientBuilder::create([]) ); } - - private function getTransport(Client $client): TransportInterface - { - $property = new \ReflectionProperty(Client::class, 'transport'); - - $property->setAccessible(true); - $value = $property->getValue($client); - $property->setAccessible(false); - - return $value; - } } final class StubIntegration implements IntegrationInterface diff --git a/tests/ClientTest.php b/tests/ClientTest.php index 749b25836..b76c1c6db 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -4,8 +4,6 @@ namespace Sentry\Tests; -use GuzzleHttp\Promise\FulfilledPromise; -use GuzzleHttp\Promise\PromiseInterface; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; @@ -18,15 +16,14 @@ use Sentry\Frame; use Sentry\Integration\IntegrationInterface; use Sentry\Options; -use Sentry\Response; -use Sentry\ResponseStatus; use Sentry\Serializer\RepresentationSerializerInterface; use Sentry\Serializer\Serializer; use Sentry\Serializer\SerializerInterface; use Sentry\Severity; use Sentry\Stacktrace; use Sentry\State\Scope; -use Sentry\Transport\TransportFactoryInterface; +use Sentry\Transport\Result; +use Sentry\Transport\ResultStatus; use Sentry\Transport\TransportInterface; use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; @@ -94,12 +91,12 @@ public function testCaptureMessage(): void return true; })) - ->willReturnCallback(static function (Event $event): FulfilledPromise { - return new FulfilledPromise(new Response(ResponseStatus::success(), $event)); + ->willReturnCallback(static function (Event $event): Result { + return new Result(ResultStatus::success(), $event); }); $client = ClientBuilder::create() - ->setTransportFactory($this->createTransportFactory($transport)) + ->setTransport($transport) ->getClient(); $this->assertNotNull($client->captureMessage('foo', Severity::fatal())); @@ -145,12 +142,12 @@ public function testCaptureException(): void return true; })) - ->willReturnCallback(static function (Event $event): FulfilledPromise { - return new FulfilledPromise(new Response(ResponseStatus::success(), $event)); + ->willReturnCallback(static function (Event $event): Result { + return new Result(ResultStatus::success(), $event); }); $client = ClientBuilder::create() - ->setTransportFactory($this->createTransportFactory($transport)) + ->setTransport($transport) ->getClient(); $this->assertNotNull($client->captureException($exception)); @@ -207,12 +204,12 @@ public function testCaptureEvent(array $options, Event $event, Event $expectedEv $transport->expects($this->once()) ->method('send') ->with($expectedEvent) - ->willReturnCallback(static function (Event $event): FulfilledPromise { - return new FulfilledPromise(new Response(ResponseStatus::success(), $event)); + ->willReturnCallback(static function (Event $event): Result { + return new Result(ResultStatus::success(), $event); }); $client = ClientBuilder::create($options) - ->setTransportFactory($this->createTransportFactory($transport)) + ->setTransport($transport) ->getClient(); $this->assertSame($event->getId(), $client->captureEvent($event)); @@ -331,12 +328,12 @@ public function testCaptureEventAttachesStacktraceAccordingToAttachStacktraceOpt return true; })) - ->willReturnCallback(static function (Event $event): FulfilledPromise { - return new FulfilledPromise(new Response(ResponseStatus::success(), $event)); + ->willReturnCallback(static function (Event $event): Result { + return new Result(ResultStatus::success(), $event); }); $client = ClientBuilder::create(['attach_stacktrace' => $attachStacktraceOption]) - ->setTransportFactory($this->createTransportFactory($transport)) + ->setTransport($transport) ->getClient(); $this->assertNotNull($client->captureEvent(Event::createEvent(), $hint)); @@ -386,12 +383,12 @@ public function testCaptureEventPrefersExplicitStacktrace(): void ->with($this->callback(static function (Event $event) use ($stacktrace): bool { return $stacktrace === $event->getStacktrace(); })) - ->willReturnCallback(static function (Event $event): FulfilledPromise { - return new FulfilledPromise(new Response(ResponseStatus::success(), $event)); + ->willReturnCallback(static function (Event $event): Result { + return new Result(ResultStatus::success(), $event); }); $client = ClientBuilder::create(['attach_stacktrace' => true]) - ->setTransportFactory($this->createTransportFactory($transport)) + ->setTransport($transport) ->getClient(); $this->assertNotNull($client->captureEvent(Event::createEvent(), EventHint::fromArray([ @@ -413,12 +410,12 @@ public function testCaptureLastError(): void return true; })) - ->willReturnCallback(static function (Event $event): FulfilledPromise { - return new FulfilledPromise(new Response(ResponseStatus::success(), $event)); + ->willReturnCallback(static function (Event $event): Result { + return new Result(ResultStatus::success(), $event); }); $client = ClientBuilder::create(['dsn' => 'http://public:secret@example.com/1']) - ->setTransportFactory($this->createTransportFactory($transport)) + ->setTransport($transport) ->getClient(); @trigger_error('foo', \E_USER_NOTICE); @@ -464,7 +461,7 @@ public function testCaptureLastErrorDoesNothingWhenThereIsNoError(): void ->with($this->anything()); $client = ClientBuilder::create(['dsn' => 'http://public:secret@example.com/1']) - ->setTransportFactory($this->createTransportFactory($transport)) + ->setTransport($transport) ->getClient(); error_clear_last(); @@ -553,7 +550,7 @@ public function testProcessEventDiscardsEventWhenSampleRateOptionIsZero(): void })); $client = ClientBuilder::create(['sample_rate' => 0]) - ->setTransportFactory($this->createTransportFactory($transport)) + ->setTransport($transport) ->setLogger($logger) ->getClient(); @@ -568,7 +565,7 @@ public function testProcessEventCapturesEventWhenSampleRateOptionIsAboveZero(): ->with($this->anything()); $client = ClientBuilder::create(['sample_rate' => 1]) - ->setTransportFactory($this->createTransportFactory($transport)) + ->setTransport($transport) ->getClient(); $client->captureEvent(Event::createEvent()); @@ -700,12 +697,12 @@ public function testAttachStacktrace(): void return null !== $result; })) - ->willReturnCallback(static function (Event $event): FulfilledPromise { - return new FulfilledPromise(new Response(ResponseStatus::success(), $event)); + ->willReturnCallback(static function (Event $event): Result { + return new Result(ResultStatus::success(), $event); }); $client = ClientBuilder::create(['attach_stacktrace' => true]) - ->setTransportFactory($this->createTransportFactory($transport)) + ->setTransport($transport) ->getClient(); $this->assertNotNull($client->captureMessage('test')); @@ -718,16 +715,15 @@ public function testFlush(): void $transport->expects($this->once()) ->method('close') ->with(10) - ->willReturn(new FulfilledPromise(true)); + ->willReturn(new Result(ResultStatus::success())); $client = ClientBuilder::create() - ->setTransportFactory($this->createTransportFactory($transport)) + ->setTransport($transport) ->getClient(); - $promise = $client->flush(10); + $response = $client->flush(10); - $this->assertSame(PromiseInterface::FULFILLED, $promise->getState()); - $this->assertTrue($promise->wait()); + $this->assertSame(ResultStatus::success(), $response->getStatus()); } public function testBuildEventInCLIDoesntSetTransaction(): void @@ -992,24 +988,4 @@ public static function getCspReportUrlDataProvider(): \Generator 'https://example.com/api/1/security/?sentry_key=public&sentry_release=dev-release&sentry_environment=development', ]; } - - private function createTransportFactory(TransportInterface $transport): TransportFactoryInterface - { - return new class($transport) implements TransportFactoryInterface { - /** - * @var TransportInterface - */ - private $transport; - - public function __construct(TransportInterface $transport) - { - $this->transport = $transport; - } - - public function create(Options $options): TransportInterface - { - return $this->transport; - } - }; - } } diff --git a/tests/HttpClient/Authentication/SentryAuthenticationTest.php b/tests/HttpClient/Authentication/SentryAuthenticationTest.php deleted file mode 100644 index 726903a10..000000000 --- a/tests/HttpClient/Authentication/SentryAuthenticationTest.php +++ /dev/null @@ -1,61 +0,0 @@ - 'http://public:secret@example.com/sentry/1']); - $authentication = new SentryAuthentication($configuration, 'sentry.php.test', '1.2.3'); - $request = new Request('POST', 'http://www.example.com', []); - $expectedHeader = sprintf( - 'Sentry sentry_version=%s, sentry_client=%s, sentry_key=public, sentry_secret=secret', - Client::PROTOCOL_VERSION, - 'sentry.php.test/1.2.3' - ); - - $this->assertFalse($request->hasHeader('X-Sentry-Auth')); - - $request = $authentication->authenticate($request); - - $this->assertTrue($request->hasHeader('X-Sentry-Auth')); - $this->assertSame($expectedHeader, $request->getHeaderLine('X-Sentry-Auth')); - } - - public function testAuthenticateWithoutSecretKey(): void - { - $configuration = new Options(['dsn' => 'http://public@example.com/sentry/1']); - $authentication = new SentryAuthentication($configuration, 'sentry.php.test', '1.2.3'); - $request = new Request('POST', 'http://www.example.com', []); - $expectedHeader = sprintf( - 'Sentry sentry_version=%s, sentry_client=%s, sentry_key=public', - Client::PROTOCOL_VERSION, - 'sentry.php.test/1.2.3' - ); - - $this->assertFalse($request->hasHeader('X-Sentry-Auth')); - - $request = $authentication->authenticate($request); - - $this->assertTrue($request->hasHeader('X-Sentry-Auth')); - $this->assertSame($expectedHeader, $request->getHeaderLine('X-Sentry-Auth')); - } - - public function testAuthenticateWithoutDsnOptionSet(): void - { - $authentication = new SentryAuthentication(new Options(), 'sentry.php.test', '1.2.3'); - $request = new Request('POST', 'http://www.example.com', []); - $request = $authentication->authenticate($request); - - $this->assertFalse($request->hasHeader('X-Sentry-Auth')); - } -} diff --git a/tests/HttpClient/HttpClientFactoryTest.php b/tests/HttpClient/HttpClientFactoryTest.php deleted file mode 100644 index b0218c817..000000000 --- a/tests/HttpClient/HttpClientFactoryTest.php +++ /dev/null @@ -1,104 +0,0 @@ -create(new Options([ - 'dsn' => 'http://public@example.com/sentry/1', - 'default_integrations' => false, - 'enable_compression' => $isCompressionEnabled, - ])); - - $request = Psr17FactoryDiscovery::findRequestFactory() - ->createRequest('POST', 'http://example.com/sentry/foo') - ->withBody($streamFactory->createStream('foo bar')); - - $httpClient->sendAsyncRequest($request); - - $httpRequest = $mockHttpClient->getLastRequest(); - - $this->assertSame('http://example.com/sentry/foo', (string) $httpRequest->getUri()); - $this->assertSame('sentry.php.test/1.2.3', $httpRequest->getHeaderLine('User-Agent')); - $this->assertSame('Sentry sentry_version=7, sentry_client=sentry.php.test/1.2.3, sentry_key=public', $httpRequest->getHeaderLine('X-Sentry-Auth')); - $this->assertSame($expectedRequestBody, (string) $httpRequest->getBody()); - } - - public static function createDataProvider(): \Generator - { - yield [ - false, - 'foo bar', - ]; - - yield [ - true, - gzcompress('foo bar', -1, \ZLIB_ENCODING_GZIP), - ]; - } - - public function testCreateThrowsIfDsnOptionIsNotConfigured(): void - { - $httpClientFactory = new HttpClientFactory( - null, - null, - Psr17FactoryDiscovery::findStreamFactory(), - null, - 'sentry.php.test', - '1.2.3' - ); - - $this->expectException(\RuntimeException::class); - $this->expectExceptionMessage('Cannot create an HTTP client without the Sentry DSN set in the options.'); - - $httpClientFactory->create(new Options(['default_integrations' => false])); - } - - public function testCreateThrowsIfHttpProxyOptionIsUsedWithCustomHttpClient(): void - { - $httpClientFactory = new HttpClientFactory( - null, - null, - Psr17FactoryDiscovery::findStreamFactory(), - $this->createMock(HttpAsyncClientInterface::class), - 'sentry.php.test', - '1.2.3' - ); - - $this->expectException(\RuntimeException::class); - $this->expectExceptionMessage('The "http_proxy" option does not work together with a custom HTTP client.'); - - $httpClientFactory->create(new Options([ - 'dsn' => 'http://public@example.com/sentry/1', - 'default_integrations' => false, - 'http_proxy' => 'http://example.com', - ])); - } -} diff --git a/tests/HttpClient/Plugin/GzipEncoderPluginTest.php b/tests/HttpClient/Plugin/GzipEncoderPluginTest.php deleted file mode 100644 index 0e5423015..000000000 --- a/tests/HttpClient/Plugin/GzipEncoderPluginTest.php +++ /dev/null @@ -1,40 +0,0 @@ -createMock(PromiseInterface::class); - $request = Psr17FactoryDiscovery::findRequestFactory() - ->createRequest('POST', 'http://www.example.com') - ->withBody($streamFactory->createStream('foo')); - - $this->assertSame('foo', (string) $request->getBody()); - $this->assertSame($expectedPromise, $plugin->handleRequest( - $request, - function (RequestInterface $requestArg) use ($expectedPromise): PromiseInterface { - $this->assertSame('gzip', $requestArg->getHeaderLine('Content-Encoding')); - $this->assertSame(gzcompress('foo', -1, \ZLIB_ENCODING_GZIP), (string) $requestArg->getBody()); - - return $expectedPromise; - }, - static function (): void {} - )); - } -} diff --git a/tests/Transport/DefaultTransportFactoryTest.php b/tests/Transport/DefaultTransportFactoryTest.php deleted file mode 100644 index 8ef47ddd4..000000000 --- a/tests/Transport/DefaultTransportFactoryTest.php +++ /dev/null @@ -1,50 +0,0 @@ -createMock(StreamFactoryInterface::class), - $this->createMock(RequestFactoryInterface::class), - $this->createMock(HttpClientFactoryInterface::class) - ); - - $this->assertInstanceOf(NullTransport::class, $factory->create(new Options())); - } - - public function testCreateReturnsHttpTransportWhenDsnOptionIsConfigured(): void - { - $options = new Options(['dsn' => 'http://public@example.com/sentry/1']); - - /** @var HttpClientFactoryInterface&MockObject $httpClientFactory */ - $httpClientFactory = $this->createMock(HttpClientFactoryInterface::class); - $httpClientFactory->expects($this->once()) - ->method('create') - ->with($options) - ->willReturn($this->createMock(HttpAsyncClientInterface::class)); - - $factory = new DefaultTransportFactory( - $this->createMock(StreamFactoryInterface::class), - $this->createMock(RequestFactoryInterface::class), - $httpClientFactory - ); - - $this->assertInstanceOf(HttpTransport::class, $factory->create($options)); - } -} diff --git a/tests/Transport/HttpTransportTest.php b/tests/Transport/HttpTransportTest.php index 95483aef3..72c17d5b4 100644 --- a/tests/Transport/HttpTransportTest.php +++ b/tests/Transport/HttpTransportTest.php @@ -4,26 +4,15 @@ namespace Sentry\Tests\Transport; -use GuzzleHttp\Promise\PromiseInterface; -use GuzzleHttp\Promise\RejectionException; -use GuzzleHttp\Psr7\Request; -use GuzzleHttp\Psr7\Response; -use GuzzleHttp\Psr7\Utils; -use Http\Client\HttpAsyncClient as HttpAsyncClientInterface; -use Http\Promise\FulfilledPromise as HttpFullfilledPromise; -use Http\Promise\RejectedPromise as HttpRejectedPromise; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Psr\Http\Message\RequestFactoryInterface; -use Psr\Http\Message\StreamFactoryInterface; -use Psr\Http\Message\StreamInterface; use Psr\Log\LoggerInterface; -use Sentry\Dsn; use Sentry\Event; -use Sentry\Options; -use Sentry\ResponseStatus; +use Sentry\HttpClient\HttpClientInterface; +use Sentry\HttpClient\Response; use Sentry\Serializer\PayloadSerializerInterface; use Sentry\Transport\HttpTransport; +use Sentry\Transport\ResultStatus; use Symfony\Bridge\PhpUnit\ClockMock; final class HttpTransportTest extends TestCase @@ -33,11 +22,6 @@ final class HttpTransportTest extends TestCase */ private $httpClient; - /** - * @var MockObject&StreamFactoryInterface - */ - private $streamFactory; - /** * @var MockObject&RequestFactoryInterface */ @@ -50,82 +34,15 @@ final class HttpTransportTest extends TestCase protected function setUp(): void { - $this->httpClient = $this->createMock(HttpAsyncClientInterface::class); - $this->streamFactory = $this->createMock(StreamFactoryInterface::class); - $this->requestFactory = $this->createMock(RequestFactoryInterface::class); + $this->httpClient = $this->createMock(HttpClientInterface::class); $this->payloadSerializer = $this->createMock(PayloadSerializerInterface::class); } - public function testSendThrowsIfDsnOptionIsNotSet(): void - { - $transport = new HttpTransport( - new Options(), - $this->httpClient, - $this->streamFactory, - $this->requestFactory, - $this->payloadSerializer - ); - - $this->expectException(\RuntimeException::class); - $this->expectExceptionMessage('The DSN option must be set to use the "Sentry\Transport\HttpTransport" transport.'); - - $transport->send(Event::createEvent()); - } - - public function testSendTransactionAsEnvelope(): void - { - $dsn = Dsn::createFromString('http://public@example.com/sentry/1'); - $event = Event::createTransaction(); - - $this->payloadSerializer->expects($this->once()) - ->method('serialize') - ->with($event) - ->willReturn('{"foo":"bar"}'); - - $this->requestFactory->expects($this->once()) - ->method('createRequest') - ->with('POST', $dsn->getEnvelopeApiEndpointUrl()) - ->willReturn(new Request('POST', 'http://www.example.com')); - - $this->streamFactory->expects($this->once()) - ->method('createStream') - ->with('{"foo":"bar"}') - ->willReturnCallback(static function (string $content): StreamInterface { - return Utils::streamFor($content); - }); - - $this->httpClient->expects($this->once()) - ->method('sendAsyncRequest') - ->with($this->callback(function (Request $requestArg): bool { - if ('application/x-sentry-envelope' !== $requestArg->getHeaderLine('Content-Type')) { - return false; - } - - if ('{"foo":"bar"}' !== $requestArg->getBody()->getContents()) { - return false; - } - - return true; - })) - ->willReturn(new HttpFullfilledPromise(new Response())); - - $transport = new HttpTransport( - new Options(['dsn' => $dsn]), - $this->httpClient, - $this->streamFactory, - $this->requestFactory, - $this->payloadSerializer - ); - - $transport->send($event); - } - /** * @dataProvider sendDataProvider */ - public function testSend(int $httpStatusCode, string $expectedPromiseStatus, ResponseStatus $expectedResponseStatus): void + public function testSend(int $httpStatusCode, ResultStatus $expectedResultStatus, bool $expectEventReturned): void { - $dsn = Dsn::createFromString('http://public@example.com/sentry/1'); $event = Event::createEvent(); $this->payloadSerializer->expects($this->once()) @@ -133,72 +50,52 @@ public function testSend(int $httpStatusCode, string $expectedPromiseStatus, Res ->with($event) ->willReturn('{"foo":"bar"}'); - $this->streamFactory->expects($this->once()) - ->method('createStream') - ->with('{"foo":"bar"}') - ->willReturnCallback(static function (string $content): StreamInterface { - return Utils::streamFor($content); - }); - - $this->requestFactory->expects($this->once()) - ->method('createRequest') - ->with('POST', $dsn->getStoreApiEndpointUrl()) - ->willReturn(new Request('POST', 'http://www.example.com')); - $this->httpClient->expects($this->once()) - ->method('sendAsyncRequest') - ->with($this->callback(function (Request $requestArg): bool { - if ('application/json' !== $requestArg->getHeaderLine('Content-Type')) { - return false; - } - - if ('{"foo":"bar"}' !== $requestArg->getBody()->getContents()) { - return false; - } - - return true; - })) - ->willReturn(new HttpFullfilledPromise(new Response($httpStatusCode))); + ->method('sendRequest') + ->willReturn(new Response($httpStatusCode, [], '')); $transport = new HttpTransport( - new Options(['dsn' => 'http://public@example.com/sentry/1']), $this->httpClient, - $this->streamFactory, - $this->requestFactory, $this->payloadSerializer ); - $promise = $transport->send($event); + $result = $transport->send($event); - try { - $promiseResult = $promise->wait(); - } catch (RejectionException $exception) { - $promiseResult = $exception->getReason(); + $this->assertSame($expectedResultStatus, $result->getStatus()); + if ($expectEventReturned) { + $this->assertSame($event, $result->getEvent()); } - - $this->assertSame($expectedPromiseStatus, $promise->getState()); - $this->assertSame($expectedResponseStatus, $promiseResult->getStatus()); - $this->assertSame($event, $promiseResult->getEvent()); } public static function sendDataProvider(): iterable { yield [ 200, - PromiseInterface::FULFILLED, - ResponseStatus::success(), + ResultStatus::success(), + true, + ]; + + yield [ + 401, + ResultStatus::invalid(), + false, + ]; + + yield [ + 429, + ResultStatus::rateLimit(), + false, ]; yield [ 500, - PromiseInterface::REJECTED, - ResponseStatus::failed(), + ResultStatus::failed(), + false, ]; } - public function testSendReturnsRejectedPromiseIfSendingFailedDueToHttpClientException(): void + public function testSendFailsDueToHttpClientException(): void { - $dsn = Dsn::createFromString('http://public@example.com/sentry/1'); $exception = new \Exception('foo'); $event = Event::createEvent(); @@ -213,52 +110,28 @@ public function testSendReturnsRejectedPromiseIfSendingFailedDueToHttpClientExce ->with($event) ->willReturn('{"foo":"bar"}'); - $this->requestFactory->expects($this->once()) - ->method('createRequest') - ->with('POST', $dsn->getStoreApiEndpointUrl()) - ->willReturn(new Request('POST', 'http://www.example.com')); - - $this->streamFactory->expects($this->once()) - ->method('createStream') - ->with('{"foo":"bar"}') - ->willReturnCallback(static function (string $content): StreamInterface { - return Utils::streamFor($content); - }); - $this->httpClient->expects($this->once()) - ->method('sendAsyncRequest') - ->willReturn(new HttpRejectedPromise($exception)); + ->method('sendRequest') + ->will($this->throwException($exception)); $transport = new HttpTransport( - new Options(['dsn' => $dsn]), $this->httpClient, - $this->streamFactory, - $this->requestFactory, $this->payloadSerializer, $logger ); - $promise = $transport->send($event); - - try { - $promiseResult = $promise->wait(); - } catch (RejectionException $exception) { - $promiseResult = $exception->getReason(); - } + $result = $transport->send($event); - $this->assertSame(PromiseInterface::REJECTED, $promise->getState()); - $this->assertSame(ResponseStatus::failed(), $promiseResult->getStatus()); - $this->assertSame($event, $promiseResult->getEvent()); + $this->assertSame(ResultStatus::failed(), $result->getStatus()); } /** * @group time-sensitive */ - public function testSendReturnsRejectedPromiseIfExceedingRateLimits(): void + public function testSendFailsDueToExceedingRateLimits(): void { ClockMock::withClockMock(1644105600); - $dsn = Dsn::createFromString('http://public@example.com/sentry/1'); $event = Event::createEvent(); /** @var LoggerInterface&MockObject $logger */ @@ -275,69 +148,36 @@ public function testSendReturnsRejectedPromiseIfExceedingRateLimits(): void ->with($event) ->willReturn('{"foo":"bar"}'); - $this->requestFactory->expects($this->once()) - ->method('createRequest') - ->with('POST', $dsn->getStoreApiEndpointUrl()) - ->willReturn(new Request('POST', 'http://www.example.com')); - - $this->streamFactory->expects($this->once()) - ->method('createStream') - ->with('{"foo":"bar"}') - ->willReturnCallback([Utils::class, 'streamFor']); - $this->httpClient->expects($this->once()) - ->method('sendAsyncRequest') - ->willReturn(new HttpFullfilledPromise(new Response(429, ['Retry-After' => '60']))); + ->method('sendRequest') + ->willReturn(new Response(429, ['Retry-After' => '60'], '')); $transport = new HttpTransport( - new Options(['dsn' => $dsn]), $this->httpClient, - $this->streamFactory, - $this->requestFactory, $this->payloadSerializer, $logger ); // Event should be sent, but the server should reply with a HTTP 429 - $promise = $transport->send($event); - - try { - $promiseResult = $promise->wait(); - } catch (RejectionException $exception) { - $promiseResult = $exception->getReason(); - } + $result = $transport->send($event); - $this->assertSame(PromiseInterface::REJECTED, $promise->getState()); - $this->assertSame(ResponseStatus::rateLimit(), $promiseResult->getStatus()); - $this->assertSame($event, $promiseResult->getEvent()); + $this->assertSame(ResultStatus::rateLimit(), $result->getStatus()); // Event should not be sent at all because rate-limit is in effect - $promise = $transport->send($event); - - try { - $promiseResult = $promise->wait(); - } catch (RejectionException $exception) { - $promiseResult = $exception->getReason(); - } + $result = $transport->send($event); - $this->assertSame(PromiseInterface::REJECTED, $promise->getState()); - $this->assertSame(ResponseStatus::rateLimit(), $promiseResult->getStatus()); - $this->assertSame($event, $promiseResult->getEvent()); + $this->assertSame(ResultStatus::rateLimit(), $result->getStatus()); } public function testClose(): void { $transport = new HttpTransport( - new Options(['dsn' => 'http://public@example.com/sentry/1']), - $this->createMock(HttpAsyncClientInterface::class), - $this->createMock(StreamFactoryInterface::class), - $this->createMock(RequestFactoryInterface::class), + $this->createMock(HttpClientInterface::class), $this->createMock(PayloadSerializerInterface::class) ); - $promise = $transport->close(); + $result = $transport->close(); - $this->assertSame(PromiseInterface::FULFILLED, $promise->getState()); - $this->assertTrue($promise->wait()); + $this->assertSame(ResultStatus::success(), $result->getStatus()); } } diff --git a/tests/Transport/NullTransportTest.php b/tests/Transport/NullTransportTest.php index 954e3ee0b..dbefc9add 100644 --- a/tests/Transport/NullTransportTest.php +++ b/tests/Transport/NullTransportTest.php @@ -4,11 +4,10 @@ namespace Sentry\Tests\Transport; -use GuzzleHttp\Promise\PromiseInterface; use PHPUnit\Framework\TestCase; use Sentry\Event; -use Sentry\ResponseStatus; use Sentry\Transport\NullTransport; +use Sentry\Transport\ResultStatus; final class NullTransportTest extends TestCase { @@ -26,19 +25,16 @@ public function testSend(): void { $event = Event::createEvent(); - $promise = $this->transport->send($event); - $promiseResult = $promise->wait(); + $result = $this->transport->send($event); - $this->assertSame(PromiseInterface::FULFILLED, $promise->getState()); - $this->assertSame(ResponseStatus::skipped(), $promiseResult->getStatus()); - $this->assertSame($event, $promiseResult->getEvent()); + $this->assertSame(ResultStatus::skipped(), $result->getStatus()); + $this->assertSame($event, $result->getEvent()); } public function testClose(): void { - $promise = $this->transport->close(); + $response = $this->transport->close(); - $this->assertSame(PromiseInterface::FULFILLED, $promise->getState()); - $this->assertTrue($promise->wait()); + $this->assertSame(ResultStatus::success(), $response->getStatus()); } } diff --git a/tests/Transport/RateLimiterTest.php b/tests/Transport/RateLimiterTest.php index 22c3e5354..106a7c54d 100644 --- a/tests/Transport/RateLimiterTest.php +++ b/tests/Transport/RateLimiterTest.php @@ -4,14 +4,12 @@ namespace Sentry\Tests\Transport; -use GuzzleHttp\Psr7\Response; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Psr\Http\Message\ResponseInterface; use Psr\Log\LoggerInterface; use Sentry\Event; use Sentry\EventType; -use Sentry\ResponseStatus; +use Sentry\HttpClient\Response; use Sentry\Transport\RateLimiter; use Symfony\Bridge\PhpUnit\ClockMock; @@ -39,56 +37,55 @@ protected function setUp(): void /** * @dataProvider handleResponseDataProvider */ - public function testHandleResponse(Event $event, ResponseInterface $response, ResponseStatus $responseStatus): void + public function testHandleResponse(Event $event, Response $response, int $responseStatusCode): void { ClockMock::withClockMock(1644105600); - $this->logger->expects($responseStatus === ResponseStatus::success() ? $this->never() : $this->once()) + $this->logger->expects($response->isSuccess() ? $this->never() : $this->once()) ->method('warning') ->with('Rate limited exceeded for requests of type "event", backing off until "2022-02-06T00:01:00+00:00".', ['event' => $event]); - $transportResponse = $this->rateLimiter->handleResponse($event, $response); + $rateLimiterResponse = $this->rateLimiter->handleResponse($event, $response); - $this->assertSame($responseStatus, $transportResponse->getStatus()); - $this->assertSame($event, $transportResponse->getEvent()); + $this->assertSame($responseStatusCode, $rateLimiterResponse->getStatusCode()); } public static function handleResponseDataProvider(): \Generator { yield 'Rate limits headers missing' => [ Event::createEvent(), - new Response(), - ResponseStatus::success(), + new Response(200, [], ''), + 200, ]; yield 'Back-off using X-Sentry-Rate-Limits header with single category' => [ Event::createEvent(), - new Response(429, ['X-Sentry-Rate-Limits' => '60:error:org']), - ResponseStatus::rateLimit(), + new Response(429, ['X-Sentry-Rate-Limits' => '60:error:org'], ''), + 429, ]; yield 'Back-off using X-Sentry-Rate-Limits header with multiple categories' => [ Event::createEvent(), - new Response(429, ['X-Sentry-Rate-Limits' => '60:error;transaction:org']), - ResponseStatus::rateLimit(), + new Response(429, ['X-Sentry-Rate-Limits' => '60:error;transaction:org'], ''), + 429, ]; yield 'Back-off using X-Sentry-Rate-Limits header with missing categories should lock them all' => [ Event::createEvent(), - new Response(429, ['X-Sentry-Rate-Limits' => '60::org']), - ResponseStatus::rateLimit(), + new Response(429, ['X-Sentry-Rate-Limits' => '60::org'], ''), + 429, ]; yield 'Back-off using Retry-After header with number-based value' => [ Event::createEvent(), - new Response(429, ['Retry-After' => '60']), - ResponseStatus::rateLimit(), + new Response(429, ['Retry-After' => '60'], ''), + 429, ]; yield 'Back-off using Retry-After header with date-based value' => [ Event::createEvent(), - new Response(429, ['Retry-After' => 'Sun, 02 February 2022 00:01:00 GMT']), - ResponseStatus::rateLimit(), + new Response(429, ['Retry-After' => 'Sun, 02 February 2022 00:01:00 GMT'], ''), + 429, ]; } @@ -102,7 +99,7 @@ public function testIsRateLimited(): void // Events should be rate-limited for 60 seconds, but transactions should // still be allowed to be sent - $this->rateLimiter->handleResponse(Event::createEvent(), new Response(429, ['X-Sentry-Rate-Limits' => '60:error:org'])); + $this->rateLimiter->handleResponse(Event::createEvent(), new Response(429, ['X-Sentry-Rate-Limits' => '60:error:org'], '')); $this->assertTrue($this->rateLimiter->isRateLimited(EventType::event())); $this->assertFalse($this->rateLimiter->isRateLimited(EventType::transaction())); @@ -115,7 +112,7 @@ public function testIsRateLimited(): void // Both events and transactions should be rate-limited if all categories // are - $this->rateLimiter->handleResponse(Event::createTransaction(), new Response(429, ['X-Sentry-Rate-Limits' => '60:all:org'])); + $this->rateLimiter->handleResponse(Event::createTransaction(), new Response(429, ['X-Sentry-Rate-Limits' => '60:all:org'], '')); $this->assertTrue($this->rateLimiter->isRateLimited(EventType::event())); $this->assertTrue($this->rateLimiter->isRateLimited(EventType::transaction())); diff --git a/tests/ResponseStatusTest.php b/tests/Transport/ResultStatusTest.php similarity index 53% rename from tests/ResponseStatusTest.php rename to tests/Transport/ResultStatusTest.php index 3edfba746..cea73dc00 100644 --- a/tests/ResponseStatusTest.php +++ b/tests/Transport/ResultStatusTest.php @@ -2,17 +2,17 @@ declare(strict_types=1); -namespace Sentry\Tests; +namespace Sentry\Tests\Transport; use PHPUnit\Framework\TestCase; -use Sentry\ResponseStatus; +use Sentry\Transport\ResultStatus; -final class ResponseStatusTest extends TestCase +final class ResultStatusTest extends TestCase { /** * @dataProvider toStringDataProvider */ - public function testToString(ResponseStatus $responseStatus, string $expectedStringRepresentation): void + public function testToString(ResultStatus $responseStatus, string $expectedStringRepresentation): void { $this->assertSame($expectedStringRepresentation, (string) $responseStatus); } @@ -20,32 +20,32 @@ public function testToString(ResponseStatus $responseStatus, string $expectedStr public static function toStringDataProvider(): iterable { yield [ - ResponseStatus::success(), + ResultStatus::success(), 'SUCCESS', ]; yield [ - ResponseStatus::failed(), + ResultStatus::failed(), 'FAILED', ]; yield [ - ResponseStatus::invalid(), + ResultStatus::invalid(), 'INVALID', ]; yield [ - ResponseStatus::skipped(), + ResultStatus::skipped(), 'SKIPPED', ]; yield [ - ResponseStatus::rateLimit(), + ResultStatus::rateLimit(), 'RATE_LIMIT', ]; yield [ - ResponseStatus::unknown(), + ResultStatus::unknown(), 'UNKNOWN', ]; } @@ -53,59 +53,59 @@ public static function toStringDataProvider(): iterable /** * @dataProvider createFromHttpStatusCodeDataProvider */ - public function testCreateFromHttpStatusCode(ResponseStatus $expectedResponseStatus, int $httpStatusCode): void + public function testCreateFromHttpStatusCode(ResultStatus $expectedResultStatus, int $httpStatusCode): void { - $this->assertSame($expectedResponseStatus, ResponseStatus::createFromHttpStatusCode($httpStatusCode)); + $this->assertSame($expectedResultStatus, ResultStatus::createFromHttpStatusCode($httpStatusCode)); } public static function createFromHttpStatusCodeDataProvider(): iterable { yield [ - ResponseStatus::success(), + ResultStatus::success(), 200, ]; yield [ - ResponseStatus::success(), + ResultStatus::success(), 299, ]; yield [ - ResponseStatus::rateLimit(), + ResultStatus::rateLimit(), 429, ]; yield [ - ResponseStatus::invalid(), + ResultStatus::invalid(), 400, ]; yield [ - ResponseStatus::invalid(), + ResultStatus::invalid(), 499, ]; yield [ - ResponseStatus::failed(), + ResultStatus::failed(), 500, ]; yield [ - ResponseStatus::failed(), + ResultStatus::failed(), 501, ]; yield [ - ResponseStatus::unknown(), + ResultStatus::unknown(), 199, ]; } public function testStrictComparison(): void { - $responseStatus1 = ResponseStatus::unknown(); - $responseStatus2 = ResponseStatus::unknown(); - $responseStatus3 = ResponseStatus::skipped(); + $responseStatus1 = ResultStatus::unknown(); + $responseStatus2 = ResultStatus::unknown(); + $responseStatus3 = ResultStatus::skipped(); $this->assertSame($responseStatus1, $responseStatus2); $this->assertNotSame($responseStatus1, $responseStatus3); diff --git a/tests/phpt/error_handler_captures_errors_not_silencable_on_php_8_and_up.phpt b/tests/phpt/error_handler_captures_errors_not_silencable_on_php_8_and_up.phpt index facb159b9..f52c12932 100644 --- a/tests/phpt/error_handler_captures_errors_not_silencable_on_php_8_and_up.phpt +++ b/tests/phpt/error_handler_captures_errors_not_silencable_on_php_8_and_up.phpt @@ -15,15 +15,12 @@ declare(strict_types=1); namespace Sentry\Tests; -use GuzzleHttp\Promise\FulfilledPromise; -use GuzzleHttp\Promise\PromiseInterface; use Sentry\ClientBuilder; use Sentry\Event; use Sentry\Options; -use Sentry\Response; -use Sentry\ResponseStatus; use Sentry\SentrySdk; -use Sentry\Transport\TransportFactoryInterface; +use Sentry\Transport\Result; +use Sentry\Transport\ResultStatus; use Sentry\Transport\TransportInterface; $vendor = __DIR__; @@ -34,22 +31,17 @@ while (!file_exists($vendor . '/vendor')) { require $vendor . '/vendor/autoload.php'; -$transportFactory = new class implements TransportFactoryInterface { - public function create(Options $options): TransportInterface +$transport = new class implements TransportInterface { + public function send(Event $event): Result { - return new class implements TransportInterface { - public function send(Event $event): PromiseInterface - { - echo 'Transport called' . PHP_EOL; + echo 'Transport called' . PHP_EOL; - return new FulfilledPromise(new Response(ResponseStatus::success())); - } + return new Result(ResultStatus::success()); + } - public function close(?int $timeout = null): PromiseInterface - { - return new FulfilledPromise(true); - } - }; + public function close(?int $timeout = null): Result + { + return new Result(ResultStatus::success()); } }; @@ -62,7 +54,7 @@ $options = [ ]; $client = ClientBuilder::create($options) - ->setTransportFactory($transportFactory) + ->setTransport($transport) ->getClient(); SentrySdk::getCurrentHub()->bindClient($client); diff --git a/tests/phpt/error_handler_captures_fatal_error.phpt b/tests/phpt/error_handler_captures_fatal_error.phpt index 2f684a12a..e7b457d2b 100644 --- a/tests/phpt/error_handler_captures_fatal_error.phpt +++ b/tests/phpt/error_handler_captures_fatal_error.phpt @@ -7,16 +7,13 @@ declare(strict_types=1); namespace Sentry\Tests; -use GuzzleHttp\Promise\FulfilledPromise; -use GuzzleHttp\Promise\PromiseInterface; use Sentry\ClientBuilder; use Sentry\ErrorHandler; use Sentry\Event; use Sentry\Options; -use Sentry\Response; -use Sentry\ResponseStatus; use Sentry\SentrySdk; -use Sentry\Transport\TransportFactoryInterface; +use Sentry\Transport\Result; +use Sentry\Transport\ResultStatus; use Sentry\Transport\TransportInterface; $vendor = __DIR__; @@ -27,22 +24,17 @@ while (!file_exists($vendor . '/vendor')) { require $vendor . '/vendor/autoload.php'; -$transportFactory = new class implements TransportFactoryInterface { - public function create(Options $options): TransportInterface +$transport = new class implements TransportInterface { + public function send(Event $event): Result { - return new class implements TransportInterface { - public function send(Event $event): PromiseInterface - { - echo 'Transport called' . PHP_EOL; - - return new FulfilledPromise(new Response(ResponseStatus::success())); - } - - public function close(?int $timeout = null): PromiseInterface - { - return new FulfilledPromise(true); - } - }; + echo 'Transport called' . PHP_EOL; + + return new Result(ResultStatus::success()); + } + + public function close(?int $timeout = null): Result + { + return new Result(ResultStatus::success()); } }; @@ -51,7 +43,7 @@ $options = [ ]; $client = ClientBuilder::create($options) - ->setTransportFactory($transportFactory) + ->setTransport($transport) ->getClient(); SentrySdk::getCurrentHub()->bindClient($client); diff --git a/tests/phpt/error_handler_respects_capture_silenced_errors_option.phpt b/tests/phpt/error_handler_respects_capture_silenced_errors_option.phpt index ac2c4e223..0063489b7 100644 --- a/tests/phpt/error_handler_respects_capture_silenced_errors_option.phpt +++ b/tests/phpt/error_handler_respects_capture_silenced_errors_option.phpt @@ -9,15 +9,12 @@ declare(strict_types=1); namespace Sentry\Tests; -use GuzzleHttp\Promise\FulfilledPromise; -use GuzzleHttp\Promise\PromiseInterface; use Sentry\ClientBuilder; use Sentry\Event; use Sentry\Options; -use Sentry\Response; -use Sentry\ResponseStatus; use Sentry\SentrySdk; -use Sentry\Transport\TransportFactoryInterface; +use Sentry\Transport\Result; +use Sentry\Transport\ResultStatus; use Sentry\Transport\TransportInterface; $vendor = __DIR__; @@ -28,22 +25,17 @@ while (!file_exists($vendor . '/vendor')) { require $vendor . '/vendor/autoload.php'; -$transportFactory = new class implements TransportFactoryInterface { - public function create(Options $options): TransportInterface +$transport = new class implements TransportInterface { + public function send(Event $event): Result { - return new class implements TransportInterface { - public function send(Event $event): PromiseInterface - { - echo 'Transport called' . PHP_EOL; - - return new FulfilledPromise(new Response(ResponseStatus::success())); - } - - public function close(?int $timeout = null): PromiseInterface - { - return new FulfilledPromise(true); - } - }; + echo 'Transport called' . PHP_EOL; + + return new Result(ResultStatus::success()); + } + + public function close(?int $timeout = null): Result + { + return new Result(ResultStatus::success()); } }; @@ -53,7 +45,7 @@ $options = [ ]; $client = ClientBuilder::create($options) - ->setTransportFactory($transportFactory) + ->setTransport($transport) ->getClient(); SentrySdk::getCurrentHub()->bindClient($client); diff --git a/tests/phpt/error_handler_respects_current_error_reporting_level.phpt b/tests/phpt/error_handler_respects_current_error_reporting_level.phpt index 6b4d8e93c..f6017b8a5 100644 --- a/tests/phpt/error_handler_respects_current_error_reporting_level.phpt +++ b/tests/phpt/error_handler_respects_current_error_reporting_level.phpt @@ -15,10 +15,9 @@ use GuzzleHttp\Promise\PromiseInterface; use Sentry\ClientBuilder; use Sentry\Event; use Sentry\Options; -use Sentry\Response; -use Sentry\ResponseStatus; use Sentry\SentrySdk; -use Sentry\Transport\TransportFactoryInterface; +use Sentry\Transport\Result; +use Sentry\Transport\ResultStatus; use Sentry\Transport\TransportInterface; $vendor = __DIR__; @@ -29,20 +28,17 @@ while (!file_exists($vendor . '/vendor')) { require $vendor . '/vendor/autoload.php'; -$transportFactory = new class implements TransportFactoryInterface { - public function create(Options $options): TransportInterface +$transport = new class implements TransportInterface { + public function send(Event $event): Result { - return new class implements TransportInterface { - public function send(Event $event): PromiseInterface - { - return new FulfilledPromise(new Response(ResponseStatus::success())); - } - - public function close(?int $timeout = null): PromiseInterface - { - return new FulfilledPromise(true); - } - }; + echo 'Transport called' . PHP_EOL; + + return new Result(ResultStatus::success()); + } + + public function close(?int $timeout = null): Result + { + return new Result(ResultStatus::success()); } }; @@ -57,7 +53,7 @@ $options = [ ]; $client = ClientBuilder::create($options) - ->setTransportFactory($transportFactory) + ->setTransport($transport) ->getClient(); SentrySdk::getCurrentHub()->bindClient($client); diff --git a/tests/phpt/error_handler_respects_error_types_option_regardless_of_error_reporting.phpt b/tests/phpt/error_handler_respects_error_types_option_regardless_of_error_reporting.phpt index e47750b5c..2ab9a423e 100644 --- a/tests/phpt/error_handler_respects_error_types_option_regardless_of_error_reporting.phpt +++ b/tests/phpt/error_handler_respects_error_types_option_regardless_of_error_reporting.phpt @@ -12,10 +12,9 @@ use GuzzleHttp\Promise\PromiseInterface; use Sentry\ClientBuilder; use Sentry\Event; use Sentry\Options; -use Sentry\Response; -use Sentry\ResponseStatus; use Sentry\SentrySdk; -use Sentry\Transport\TransportFactoryInterface; +use Sentry\Transport\Result; +use Sentry\Transport\ResultStatus; use Sentry\Transport\TransportInterface; $vendor = __DIR__; @@ -26,22 +25,17 @@ while (!file_exists($vendor . '/vendor')) { require $vendor . '/vendor/autoload.php'; -$transportFactory = new class implements TransportFactoryInterface { - public function create(Options $options): TransportInterface +$transport = new class implements TransportInterface { + public function send(Event $event): Result { - return new class implements TransportInterface { - public function send(Event $event): PromiseInterface - { - echo 'Transport called' . PHP_EOL; - - return new FulfilledPromise(new Response(ResponseStatus::success())); - } - - public function close(?int $timeout = null): PromiseInterface - { - return new FulfilledPromise(true); - } - }; + echo 'Transport called' . PHP_EOL; + + return new Result(ResultStatus::success()); + } + + public function close(?int $timeout = null): Result + { + return new Result(ResultStatus::success()); } }; @@ -53,7 +47,7 @@ $options = [ ]; $client = ClientBuilder::create($options) - ->setTransportFactory($transportFactory) + ->setTransport($transport) ->getClient(); SentrySdk::getCurrentHub()->bindClient($client); diff --git a/tests/phpt/error_listener_integration_respects_error_types_option.phpt b/tests/phpt/error_listener_integration_respects_error_types_option.phpt index 870a0696f..4c66a5f9a 100644 --- a/tests/phpt/error_listener_integration_respects_error_types_option.phpt +++ b/tests/phpt/error_listener_integration_respects_error_types_option.phpt @@ -15,10 +15,9 @@ use Sentry\ClientBuilder; use Sentry\Event; use Sentry\Integration\ErrorListenerIntegration; use Sentry\Options; -use Sentry\Response; -use Sentry\ResponseStatus; use Sentry\SentrySdk; -use Sentry\Transport\TransportFactoryInterface; +use Sentry\Transport\Result; +use Sentry\Transport\ResultStatus; use Sentry\Transport\TransportInterface; $vendor = __DIR__; @@ -29,22 +28,17 @@ while (!file_exists($vendor . '/vendor')) { require $vendor . '/vendor/autoload.php'; -$transportFactory = new class implements TransportFactoryInterface { - public function create(Options $options): TransportInterface +$transport = new class implements TransportInterface { + public function send(Event $event): Result { - return new class implements TransportInterface { - public function send(Event $event): PromiseInterface - { - echo 'Transport called' . PHP_EOL; + echo 'Transport called' . PHP_EOL; - return new FulfilledPromise(new Response(ResponseStatus::success())); - } + return new Result(ResultStatus::success()); + } - public function close(?int $timeout = null): PromiseInterface - { - return new FulfilledPromise(true); - } - }; + public function close(?int $timeout = null): Result + { + return new Result(ResultStatus::success()); } }; @@ -57,7 +51,7 @@ $options = new Options([ ]); $client = (new ClientBuilder($options)) - ->setTransportFactory($transportFactory) + ->setTransport($transport) ->getClient(); SentrySdk::getCurrentHub()->bindClient($client); diff --git a/tests/phpt/fatal_error_integration_captures_fatal_error.phpt b/tests/phpt/fatal_error_integration_captures_fatal_error.phpt index d264e9810..ba82e9bca 100644 --- a/tests/phpt/fatal_error_integration_captures_fatal_error.phpt +++ b/tests/phpt/fatal_error_integration_captures_fatal_error.phpt @@ -13,10 +13,9 @@ use Sentry\ClientBuilder; use Sentry\Event; use Sentry\Integration\FatalErrorListenerIntegration; use Sentry\Options; -use Sentry\Response; -use Sentry\ResponseStatus; use Sentry\SentrySdk; -use Sentry\Transport\TransportFactoryInterface; +use Sentry\Transport\Result; +use Sentry\Transport\ResultStatus; use Sentry\Transport\TransportInterface; $vendor = __DIR__; @@ -27,22 +26,17 @@ while (!file_exists($vendor . '/vendor')) { require $vendor . '/vendor/autoload.php'; -$transportFactory = new class implements TransportFactoryInterface { - public function create(Options $options): TransportInterface +$transport = new class implements TransportInterface { + public function send(Event $event): Result { - return new class implements TransportInterface { - public function send(Event $event): PromiseInterface - { - echo 'Transport called' . PHP_EOL; + echo 'Transport called' . PHP_EOL; - return new FulfilledPromise(new Response(ResponseStatus::success())); - } + return new Result(ResultStatus::success()); + } - public function close(?int $timeout = null): PromiseInterface - { - return new FulfilledPromise(true); - } - }; + public function close(?int $timeout = null): Result + { + return new Result(ResultStatus::success()); } }; @@ -54,7 +48,7 @@ $options = new Options([ ]); $client = (new ClientBuilder($options)) - ->setTransportFactory($transportFactory) + ->setTransport($transport) ->getClient(); SentrySdk::getCurrentHub()->bindClient($client); diff --git a/tests/phpt/fatal_error_integration_respects_error_types_option.phpt b/tests/phpt/fatal_error_integration_respects_error_types_option.phpt index 00db34a07..8010b35b2 100644 --- a/tests/phpt/fatal_error_integration_respects_error_types_option.phpt +++ b/tests/phpt/fatal_error_integration_respects_error_types_option.phpt @@ -13,10 +13,9 @@ use Sentry\ClientBuilder; use Sentry\Event; use Sentry\Integration\FatalErrorListenerIntegration; use Sentry\Options; -use Sentry\Response; -use Sentry\ResponseStatus; use Sentry\SentrySdk; -use Sentry\Transport\TransportFactoryInterface; +use Sentry\Transport\Result; +use Sentry\Transport\ResultStatus; use Sentry\Transport\TransportInterface; $vendor = __DIR__; @@ -27,22 +26,17 @@ while (!file_exists($vendor . '/vendor')) { require $vendor . '/vendor/autoload.php'; -$transportFactory = new class implements TransportFactoryInterface { - public function create(Options $options): TransportInterface +$transport = new class implements TransportInterface { + public function send(Event $event): Result { - return new class implements TransportInterface { - public function send(Event $event): PromiseInterface - { - echo 'Transport called (it should not have been)' . PHP_EOL; + echo 'Transport called' . PHP_EOL; - return new FulfilledPromise(new Response(ResponseStatus::success())); - } + return new Result(ResultStatus::success()); + } - public function close(?int $timeout = null): PromiseInterface - { - return new FulfilledPromise(true); - } - }; + public function close(?int $timeout = null): Result + { + return new Result(ResultStatus::success()); } }; @@ -55,7 +49,7 @@ $options = new Options([ ]); $client = (new ClientBuilder($options)) - ->setTransportFactory($transportFactory) + ->setTransport($transport) ->getClient(); SentrySdk::getCurrentHub()->bindClient($client); From aa2c5d2fffbb60f0b61cb71d15a21d5cd75ee847 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Thu, 21 Sep 2023 11:59:58 +0200 Subject: [PATCH 02/20] Add `http_ssl_verify_peer ` option --- phpstan-baseline.neon | 5 +++++ src/HttpClient/HttpClient.php | 10 +++++----- src/Options.php | 13 +++++++++++++ 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 124b73ebc..3ca6264b3 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -115,6 +115,11 @@ parameters: count: 1 path: src/Options.php + - + message: "#^Method Sentry\\\\Options\\:\\:getHttpSslVerifyPeer\\(\\) should return bool but returns mixed\\.$#" + count: 1 + path: src/Options.php + - message: "#^Method Sentry\\\\Options\\:\\:getHttpTimeout\\(\\) should return float but returns mixed\\.$#" count: 1 diff --git a/src/HttpClient/HttpClient.php b/src/HttpClient/HttpClient.php index e2df6598e..4042af9d8 100644 --- a/src/HttpClient/HttpClient.php +++ b/src/HttpClient/HttpClient.php @@ -44,16 +44,11 @@ public function sendRequest(string $requestData): Response curl_setopt($curlHandle, \CURLOPT_USERAGENT, $this->sdkIdentifier . '/' . $this->sdkVersion); curl_setopt($curlHandle, \CURLOPT_TIMEOUT, $this->options->getHttpTimeout()); curl_setopt($curlHandle, \CURLOPT_CONNECTTIMEOUT, $this->options->getHttpConnectTimeout()); - curl_setopt($curlHandle, \CURLOPT_SSL_VERIFYPEER, true); curl_setopt($curlHandle, \CURLOPT_ENCODING, ''); curl_setopt($curlHandle, \CURLOPT_POST, true); curl_setopt($curlHandle, \CURLOPT_POSTFIELDS, $requestData); curl_setopt($curlHandle, \CURLOPT_RETURNTRANSFER, true); curl_setopt($curlHandle, \CURLOPT_HEADER, true); - /** - * @TODO(michi) make this configurable - */ - curl_setopt($curlHandle, \CURLOPT_SSL_VERIFYPEER, true); /** * @TODO(michi) make this configurable * @@ -63,6 +58,11 @@ public function sendRequest(string $requestData): Response */ curl_setopt($curlHandle, \CURLOPT_HTTP_VERSION, \CURL_HTTP_VERSION_1_1); + $httpSslVerifyPeer = $this->options->getHttpSslVerifyPeer(); + if ($httpSslVerifyPeer) { + curl_setopt($curlHandle, \CURLOPT_SSL_VERIFYPEER, true); + } + $httpProxy = $this->options->getHttpProxy(); if (null !== $httpProxy) { curl_setopt($curlHandle, \CURLOPT_PROXY, $httpProxy); diff --git a/src/Options.php b/src/Options.php index a14578b83..b237f01f1 100644 --- a/src/Options.php +++ b/src/Options.php @@ -808,6 +808,18 @@ public function setHttpTimeout(float $httpTimeout): void $this->options = $this->resolver->resolve($options); } + public function getHttpSslVerifyPeer(): bool + { + return $this->options['http_ssl_verify_peer']; + } + + public function setHttpSslVerifyPeer(bool $httpSslVerifyPeer): void + { + $options = array_merge($this->options, ['http_ssl_verify_peer' => $httpSslVerifyPeer]); + + $this->options = $this->resolver->resolve($options); + } + /** * Gets whether the silenced errors should be captured or not. * @@ -969,6 +981,7 @@ private function configureOptions(OptionsResolver $resolver): void 'http_proxy_authentication' => null, 'http_connect_timeout' => self::DEFAULT_HTTP_CONNECT_TIMEOUT, 'http_timeout' => self::DEFAULT_HTTP_TIMEOUT, + 'http_ssl_verify_peer' => true, 'capture_silenced_errors' => false, 'max_request_body_size' => 'medium', 'class_serializers' => [], From 6d81fc0f3b024021b5677e1433eaa35f4bc633e9 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Thu, 21 Sep 2023 12:00:09 +0200 Subject: [PATCH 03/20] Add tests for new options --- tests/OptionsTest.php | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/OptionsTest.php b/tests/OptionsTest.php index 4f47fbbe8..fcbeb93a1 100644 --- a/tests/OptionsTest.php +++ b/tests/OptionsTest.php @@ -6,7 +6,10 @@ use PHPUnit\Framework\TestCase; use Sentry\Dsn; +use Sentry\HttpClient\HttpClient; use Sentry\Options; +use Sentry\Serializer\PayloadSerializer; +use Sentry\Transport\HttpTransport; use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException; @@ -347,6 +350,24 @@ static function (): void {}, null, ]; + yield [ + 'transport', + new HttpTransport(new HttpClient(new Options(), 'foo', 'bar'), new PayloadSerializer(new Options())), + 'getTransport', + 'setTransport', + null, + null, + ]; + + yield [ + 'http_client', + new HttpClient(new Options(), 'foo', 'bar'), + 'getHttpClient', + 'setHttpClient', + null, + null, + ]; + yield [ 'http_proxy', '127.0.0.1', @@ -356,6 +377,15 @@ static function (): void {}, null, ]; + yield [ + 'http_proxy_authentication', + 'username:password', + 'getHttpProxyAuthentication', + 'setHttpProxyAuthentication', + null, + null, + ]; + yield [ 'http_timeout', 1, @@ -392,6 +422,15 @@ static function (): void {}, null, ]; + yield [ + 'http_ssl_verify_peer', + false, + 'getHttpSslVerifyPeer', + 'setHttpSslVerifyPeer', + null, + null, + ]; + yield [ 'capture_silenced_errors', true, From dec5282ea18069519d93e3f7d327e6f1e1df91e1 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Thu, 21 Sep 2023 13:35:01 +0200 Subject: [PATCH 04/20] Refactor HttpClient and Transport options handling --- src/ClientBuilder.php | 3 ++- src/HttpClient/HttpClient.php | 32 +++++++++----------------- src/HttpClient/HttpClientInterface.php | 4 +++- src/Options.php | 4 ++-- src/Transport/HttpTransport.php | 10 +++++++- tests/OptionsTest.php | 4 ++-- tests/Transport/HttpTransportTest.php | 5 ++++ 7 files changed, 34 insertions(+), 28 deletions(-) diff --git a/src/ClientBuilder.php b/src/ClientBuilder.php index 3fffadbee..1f74d7702 100644 --- a/src/ClientBuilder.php +++ b/src/ClientBuilder.php @@ -69,8 +69,9 @@ public function __construct(Options $options = null) { $this->options = $options ?? new Options(); - $this->httpClient = $this->options->getHttpClient() ?? new HttpClient($this->options, $this->sdkIdentifier, $this->sdkVersion); + $this->httpClient = $this->options->getHttpClient() ?? new HttpClient($this->sdkIdentifier, $this->sdkVersion); $this->transport = $this->options->getTransport() ?? new HttpTransport( + $this->options, $this->httpClient, new PayloadSerializer($this->options), $this->logger diff --git a/src/HttpClient/HttpClient.php b/src/HttpClient/HttpClient.php index 4042af9d8..68e2ea7a6 100644 --- a/src/HttpClient/HttpClient.php +++ b/src/HttpClient/HttpClient.php @@ -5,15 +5,11 @@ namespace Sentry\HttpClient; use Sentry\Client; +use Sentry\Dsn; use Sentry\Options; class HttpClient implements HttpClientInterface { - /** - * @var Options - */ - protected $options; - /** * @var string The Sentry SDK identifier */ @@ -24,26 +20,25 @@ class HttpClient implements HttpClientInterface */ protected $sdkVersion; - public function __construct(Options $options, string $sdkIdentifier, string $sdkVersion) + public function __construct(string $sdkIdentifier, string $sdkVersion) { - $this->options = $options; $this->sdkIdentifier = $sdkIdentifier; $this->sdkVersion = $sdkVersion; } - public function sendRequest(string $requestData): Response + public function sendRequest(string $requestData, Options $options): Response { - $dsn = $this->options->getDsn(); + $dsn = $options->getDsn(); if (null === $dsn) { throw new \RuntimeException('The DSN option must be set to use the HttpClient.'); } $curlHandle = curl_init(); curl_setopt($curlHandle, \CURLOPT_URL, $dsn->getEnvelopeApiEndpointUrl()); - curl_setopt($curlHandle, \CURLOPT_HTTPHEADER, $this->getRequestHeaders()); + curl_setopt($curlHandle, \CURLOPT_HTTPHEADER, $this->getRequestHeaders($dsn)); curl_setopt($curlHandle, \CURLOPT_USERAGENT, $this->sdkIdentifier . '/' . $this->sdkVersion); - curl_setopt($curlHandle, \CURLOPT_TIMEOUT, $this->options->getHttpTimeout()); - curl_setopt($curlHandle, \CURLOPT_CONNECTTIMEOUT, $this->options->getHttpConnectTimeout()); + curl_setopt($curlHandle, \CURLOPT_TIMEOUT, $$options->getHttpTimeout()); + curl_setopt($curlHandle, \CURLOPT_CONNECTTIMEOUT, $options->getHttpConnectTimeout()); curl_setopt($curlHandle, \CURLOPT_ENCODING, ''); curl_setopt($curlHandle, \CURLOPT_POST, true); curl_setopt($curlHandle, \CURLOPT_POSTFIELDS, $requestData); @@ -58,17 +53,17 @@ public function sendRequest(string $requestData): Response */ curl_setopt($curlHandle, \CURLOPT_HTTP_VERSION, \CURL_HTTP_VERSION_1_1); - $httpSslVerifyPeer = $this->options->getHttpSslVerifyPeer(); + $httpSslVerifyPeer = $options->getHttpSslVerifyPeer(); if ($httpSslVerifyPeer) { curl_setopt($curlHandle, \CURLOPT_SSL_VERIFYPEER, true); } - $httpProxy = $this->options->getHttpProxy(); + $httpProxy = $options->getHttpProxy(); if (null !== $httpProxy) { curl_setopt($curlHandle, \CURLOPT_PROXY, $httpProxy); } - $httpProxyAuthentication = $this->options->getHttpProxyAuthentication(); + $httpProxyAuthentication = $options->getHttpProxyAuthentication(); if (null !== $httpProxyAuthentication) { curl_setopt($curlHandle, \CURLOPT_PROXYUSERPWD, $httpProxyAuthentication); } @@ -100,17 +95,12 @@ public function sendRequest(string $requestData): Response /** * @return string[] */ - protected function getRequestHeaders(): array + protected function getRequestHeaders(Dsn $dsn): array { $headers = [ 'Content-Type' => 'application/x-sentry-envelope', ]; - $dsn = $this->options->getDsn(); - if (null === $dsn) { - return $headers; - } - $data = [ 'sentry_version' => Client::PROTOCOL_VERSION, 'sentry_client' => $this->sdkIdentifier . '/' . $this->sdkVersion, diff --git a/src/HttpClient/HttpClientInterface.php b/src/HttpClient/HttpClientInterface.php index 58fdc510c..c3af5cd25 100644 --- a/src/HttpClient/HttpClientInterface.php +++ b/src/HttpClient/HttpClientInterface.php @@ -4,7 +4,9 @@ namespace Sentry\HttpClient; +use Sentry\Options; + interface HttpClientInterface { - public function sendRequest(string $requestData): Response; + public function sendRequest(string $requestData, Options $options): Response; } diff --git a/src/Options.php b/src/Options.php index b237f01f1..bdcc79e4c 100644 --- a/src/Options.php +++ b/src/Options.php @@ -1017,8 +1017,8 @@ private function configureOptions(OptionsResolver $resolver): void $resolver->setAllowedTypes('send_default_pii', 'bool'); $resolver->setAllowedTypes('default_integrations', 'bool'); $resolver->setAllowedTypes('max_value_length', 'int'); - $resolver->setAllowedTypes('transport', ['null', 'Sentry\\Transport\\TransportInterface']); - $resolver->setAllowedTypes('http_client', ['null', 'Sentry\\HttpCLient\\HttpCLientInterface']); + $resolver->setAllowedTypes('transport', ['null', TransportInterface::class]); + $resolver->setAllowedTypes('http_client', ['null', HttpCLientInterface::class]); $resolver->setAllowedTypes('http_proxy', ['null', 'string']); $resolver->setAllowedTypes('http_proxy_authentication', ['null', 'string']); $resolver->setAllowedTypes('http_connect_timeout', ['int', 'float']); diff --git a/src/Transport/HttpTransport.php b/src/Transport/HttpTransport.php index 203efe32d..430fad414 100644 --- a/src/Transport/HttpTransport.php +++ b/src/Transport/HttpTransport.php @@ -8,6 +8,7 @@ use Psr\Log\NullLogger; use Sentry\Event; use Sentry\HttpClient\HttpClientInterface; +use Sentry\Options; use Sentry\Serializer\PayloadSerializerInterface; /** @@ -15,6 +16,11 @@ */ class HttpTransport implements TransportInterface { + /** + * @var Options + */ + private $options; + /** * @var HttpClientInterface The HTTP client */ @@ -41,10 +47,12 @@ class HttpTransport implements TransportInterface * @param LoggerInterface|null $logger An instance of a PSR-3 logger */ public function __construct( + Options $options, HttpClientInterface $httpClient, PayloadSerializerInterface $payloadSerializer, ?LoggerInterface $logger = null ) { + $this->options = $options; $this->httpClient = $httpClient; $this->payloadSerializer = $payloadSerializer; $this->logger = $logger ?? new NullLogger(); @@ -67,7 +75,7 @@ public function send(Event $event): Result } try { - $response = $this->httpClient->sendRequest($this->payloadSerializer->serialize($event)); + $response = $this->httpClient->sendRequest($this->payloadSerializer->serialize($event), $this->options); } catch (\Throwable $exception) { $this->logger->error( sprintf('Failed to send the event to Sentry. Reason: "%s".', $exception->getMessage()), diff --git a/tests/OptionsTest.php b/tests/OptionsTest.php index fcbeb93a1..b27dc02f7 100644 --- a/tests/OptionsTest.php +++ b/tests/OptionsTest.php @@ -352,7 +352,7 @@ static function (): void {}, yield [ 'transport', - new HttpTransport(new HttpClient(new Options(), 'foo', 'bar'), new PayloadSerializer(new Options())), + new HttpTransport(new Options(), new HttpClient('foo', 'bar'), new PayloadSerializer(new Options())), 'getTransport', 'setTransport', null, @@ -361,7 +361,7 @@ static function (): void {}, yield [ 'http_client', - new HttpClient(new Options(), 'foo', 'bar'), + new HttpClient('foo', 'bar'), 'getHttpClient', 'setHttpClient', null, diff --git a/tests/Transport/HttpTransportTest.php b/tests/Transport/HttpTransportTest.php index 72c17d5b4..c9532a50d 100644 --- a/tests/Transport/HttpTransportTest.php +++ b/tests/Transport/HttpTransportTest.php @@ -10,6 +10,7 @@ use Sentry\Event; use Sentry\HttpClient\HttpClientInterface; use Sentry\HttpClient\Response; +use Sentry\Options; use Sentry\Serializer\PayloadSerializerInterface; use Sentry\Transport\HttpTransport; use Sentry\Transport\ResultStatus; @@ -55,6 +56,7 @@ public function testSend(int $httpStatusCode, ResultStatus $expectedResultStatus ->willReturn(new Response($httpStatusCode, [], '')); $transport = new HttpTransport( + new Options(), $this->httpClient, $this->payloadSerializer ); @@ -115,6 +117,7 @@ public function testSendFailsDueToHttpClientException(): void ->will($this->throwException($exception)); $transport = new HttpTransport( + new Options(), $this->httpClient, $this->payloadSerializer, $logger @@ -153,6 +156,7 @@ public function testSendFailsDueToExceedingRateLimits(): void ->willReturn(new Response(429, ['Retry-After' => '60'], '')); $transport = new HttpTransport( + new Options(), $this->httpClient, $this->payloadSerializer, $logger @@ -172,6 +176,7 @@ public function testSendFailsDueToExceedingRateLimits(): void public function testClose(): void { $transport = new HttpTransport( + new Options(), $this->createMock(HttpClientInterface::class), $this->createMock(PayloadSerializerInterface::class) ); From 4c6ea44909c0503c198deaab4397cfced5578368 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Thu, 21 Sep 2023 15:17:44 +0200 Subject: [PATCH 05/20] Fix $$ --- src/HttpClient/HttpClient.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/HttpClient/HttpClient.php b/src/HttpClient/HttpClient.php index 68e2ea7a6..c076c5eb9 100644 --- a/src/HttpClient/HttpClient.php +++ b/src/HttpClient/HttpClient.php @@ -37,7 +37,7 @@ public function sendRequest(string $requestData, Options $options): Response curl_setopt($curlHandle, \CURLOPT_URL, $dsn->getEnvelopeApiEndpointUrl()); curl_setopt($curlHandle, \CURLOPT_HTTPHEADER, $this->getRequestHeaders($dsn)); curl_setopt($curlHandle, \CURLOPT_USERAGENT, $this->sdkIdentifier . '/' . $this->sdkVersion); - curl_setopt($curlHandle, \CURLOPT_TIMEOUT, $$options->getHttpTimeout()); + curl_setopt($curlHandle, \CURLOPT_TIMEOUT, $options->getHttpTimeout()); curl_setopt($curlHandle, \CURLOPT_CONNECTTIMEOUT, $options->getHttpConnectTimeout()); curl_setopt($curlHandle, \CURLOPT_ENCODING, ''); curl_setopt($curlHandle, \CURLOPT_POST, true); From 22c9409e45ad769b1d9943a137a5e64b40510424 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Thu, 21 Sep 2023 15:24:39 +0200 Subject: [PATCH 06/20] Add argument doc --- src/Transport/HttpTransport.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Transport/HttpTransport.php b/src/Transport/HttpTransport.php index 430fad414..efb9505c4 100644 --- a/src/Transport/HttpTransport.php +++ b/src/Transport/HttpTransport.php @@ -42,6 +42,7 @@ class HttpTransport implements TransportInterface private $rateLimiter; /** + * @param Options $options The options * @param HttpClientInterface $httpClient The HTTP client * @param PayloadSerializerInterface $payloadSerializer The event serializer * @param LoggerInterface|null $logger An instance of a PSR-3 logger From e3561fce076173b3cc8b9b4bdc3ead88a954893e Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Mon, 9 Oct 2023 20:39:18 +0200 Subject: [PATCH 07/20] Mark ClientBuilder as internal --- src/ClientBuilder.php | 55 ++++++++------------- src/ClientBuilderInterface.php | 87 ---------------------------------- 2 files changed, 21 insertions(+), 121 deletions(-) delete mode 100644 src/ClientBuilderInterface.php diff --git a/src/ClientBuilder.php b/src/ClientBuilder.php index 1f74d7702..aaefee950 100644 --- a/src/ClientBuilder.php +++ b/src/ClientBuilder.php @@ -14,11 +14,11 @@ use Sentry\Transport\TransportInterface; /** - * The default implementation of {@link ClientBuilderInterface}. + * A configurable builder for Client objects. * - * @author Stefano Arlandini + * @internal */ -final class ClientBuilder implements ClientBuilderInterface +final class ClientBuilder { /** * @var Options The client options @@ -79,65 +79,47 @@ public function __construct(Options $options = null) } /** - * {@inheritdoc} + * @param array $options The client options, in naked array form */ - public static function create(array $options = []): ClientBuilderInterface + public static function create(array $options = []): ClientBuilder { return new self(new Options($options)); } - /** - * {@inheritdoc} - */ public function getOptions(): Options { return $this->options; } - /** - * {@inheritdoc} - */ - public function setSerializer(SerializerInterface $serializer): ClientBuilderInterface + public function setSerializer(SerializerInterface $serializer): ClientBuilder { $this->serializer = $serializer; return $this; } - /** - * {@inheritdoc} - */ - public function setRepresentationSerializer(RepresentationSerializerInterface $representationSerializer): ClientBuilderInterface + public function setRepresentationSerializer(RepresentationSerializerInterface $representationSerializer): ClientBuilder { $this->representationSerializer = $representationSerializer; return $this; } - /** - * {@inheritdoc} - */ - public function setLogger(LoggerInterface $logger): ClientBuilderInterface + public function setLogger(LoggerInterface $logger): ClientBuilder { $this->logger = $logger; return $this; } - /** - * {@inheritdoc} - */ - public function setSdkIdentifier(string $sdkIdentifier): ClientBuilderInterface + public function setSdkIdentifier(string $sdkIdentifier): ClientBuilder { $this->sdkIdentifier = $sdkIdentifier; return $this; } - /** - * {@inheritdoc} - */ - public function setSdkVersion(string $sdkVersion): ClientBuilderInterface + public function setSdkVersion(string $sdkVersion): ClientBuilder { $this->sdkVersion = $sdkVersion; @@ -149,7 +131,7 @@ public function getTransport(): TransportInterface return $this->transport; } - public function setTransport(TransportInterface $transport): ClientBuilderInterface + public function setTransport(TransportInterface $transport): ClientBuilder { $this->transport = $transport; @@ -161,18 +143,23 @@ public function getHttpClient(): HttpClientInterface return $this->httpClient; } - public function setHttpClient(HttpClientInterface $httpClient): ClientBuilderInterface + public function setHttpClient(HttpClientInterface $httpClient): ClientBuilder { $this->httpClient = $httpClient; return $this; } - /** - * {@inheritdoc} - */ public function getClient(): ClientInterface { - return new Client($this->options, $this->transport, $this->sdkIdentifier, $this->sdkVersion, $this->serializer, $this->representationSerializer, $this->logger); + return new Client( + $this->options, + $this->transport, + $this->sdkIdentifier, + $this->sdkVersion, + $this->serializer, + $this->representationSerializer, + $this->logger + ); } } diff --git a/src/ClientBuilderInterface.php b/src/ClientBuilderInterface.php deleted file mode 100644 index 8d3fae1b6..000000000 --- a/src/ClientBuilderInterface.php +++ /dev/null @@ -1,87 +0,0 @@ - - */ -interface ClientBuilderInterface -{ - /** - * Creates a new instance of this builder. - * - * @param array $options The client options, in naked array form - * - * @return static - */ - public static function create(array $options = []): self; - - /** - * The options that will be used to create the {@see Client}. - */ - public function getOptions(): Options; - - /** - * Gets the instance of the client built using the configured options. - */ - public function getClient(): ClientInterface; - - /** - * Sets a serializer instance to be injected as a dependency of the client. - * - * @param SerializerInterface $serializer The serializer to be used by the client to fill the events - * - * @return $this - */ - public function setSerializer(SerializerInterface $serializer): self; - - /** - * Sets a representation serializer instance to be injected as a dependency of the client. - * - * @param RepresentationSerializerInterface $representationSerializer The representation serializer, used to serialize function - * arguments in stack traces, to have string representation - * of non-string values - * - * @return $this - */ - public function setRepresentationSerializer(RepresentationSerializerInterface $representationSerializer): self; - - /** - * Sets a PSR-3 logger to log internal debug messages. - * - * @param LoggerInterface $logger The logger instance - * - * @return $this - */ - public function setLogger(LoggerInterface $logger): ClientBuilderInterface; - - /** - * Sets the SDK identifier to be passed onto {@see Event} and HTTP User-Agent header. - * - * @param string $sdkIdentifier The SDK identifier to be sent in {@see Event} and HTTP User-Agent headers - * - * @return $this - * - * @internal - */ - public function setSdkIdentifier(string $sdkIdentifier): self; - - /** - * Sets the SDK version to be passed onto {@see Event} and HTTP User-Agent header. - * - * @param string $sdkVersion The version of the SDK in use, to be sent alongside the SDK identifier - * - * @return $this - * - * @internal - */ - public function setSdkVersion(string $sdkVersion): self; -} From fd39f7e02f6f8e0a747f1b7f75b48c0bda1fe1ae Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Mon, 9 Oct 2023 20:39:26 +0200 Subject: [PATCH 08/20] Mark Result as final --- src/Transport/Result.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Transport/Result.php b/src/Transport/Result.php index 7d5061bc4..881624f11 100644 --- a/src/Transport/Result.php +++ b/src/Transport/Result.php @@ -10,7 +10,7 @@ * This class contains the details of the sending operation of an event, e.g. * if it was sent successfully or if it was skipped because of some reason. */ -class Result +final class Result { /** * @var ResultStatus The status of the sending operation of the event From fd4f85baf205d87cd422cadecac7795c2babcc0d Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Tue, 10 Oct 2023 10:53:19 +0200 Subject: [PATCH 09/20] Cleanup HttpClient --- src/HttpClient/HttpClient.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/HttpClient/HttpClient.php b/src/HttpClient/HttpClient.php index c076c5eb9..f9544c85b 100644 --- a/src/HttpClient/HttpClient.php +++ b/src/HttpClient/HttpClient.php @@ -8,6 +8,9 @@ use Sentry\Dsn; use Sentry\Options; +/** + * @internal + */ class HttpClient implements HttpClientInterface { /** @@ -45,8 +48,6 @@ public function sendRequest(string $requestData, Options $options): Response curl_setopt($curlHandle, \CURLOPT_RETURNTRANSFER, true); curl_setopt($curlHandle, \CURLOPT_HEADER, true); /** - * @TODO(michi) make this configurable - * * If we add support for CURL_HTTP_VERSION_2_0, we need * case-insensitive header handling, as HTTP 2.0 headers * are all lowercase. From b60d22c9f78880d19506c229c7cb3f95bc393e37 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Tue, 10 Oct 2023 10:53:29 +0200 Subject: [PATCH 10/20] Clean up Response --- src/HttpClient/Response.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/HttpClient/Response.php b/src/HttpClient/Response.php index b6b8710c5..2c411dfb1 100644 --- a/src/HttpClient/Response.php +++ b/src/HttpClient/Response.php @@ -4,7 +4,10 @@ namespace Sentry\HttpClient; -class Response +/** + * @internal + */ +final class Response { /** * @var int The HTTP status code From 5b475dcda42bfd396c7976d06fa6b5f6467f126f Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Tue, 10 Oct 2023 10:53:42 +0200 Subject: [PATCH 11/20] Add HttpTransport test --- tests/Transport/HttpTransportTest.php | 36 +++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/tests/Transport/HttpTransportTest.php b/tests/Transport/HttpTransportTest.php index c9532a50d..077dc6865 100644 --- a/tests/Transport/HttpTransportTest.php +++ b/tests/Transport/HttpTransportTest.php @@ -23,11 +23,6 @@ final class HttpTransportTest extends TestCase */ private $httpClient; - /** - * @var MockObject&RequestFactoryInterface - */ - private $requestFactory; - /** * @var MockObject&PayloadSerializerInterface */ @@ -128,6 +123,37 @@ public function testSendFailsDueToHttpClientException(): void $this->assertSame(ResultStatus::failed(), $result->getStatus()); } + public function testSendFailsDueToCulrError(): void + { + $event = Event::createEvent(); + + /** @var LoggerInterface&MockObject $logger */ + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($this->once()) + ->method('error') + ->with('Failed to send the event to Sentry. Reason: "cURL Error (6) Could not resolve host: example.com".', ['event' => $event]); + + $this->payloadSerializer->expects($this->once()) + ->method('serialize') + ->with($event) + ->willReturn('{"foo":"bar"}'); + + $this->httpClient->expects($this->once()) + ->method('sendRequest') + ->willReturn(new Response(0, [], 'cURL Error (6) Could not resolve host: example.com')); + + $transport = new HttpTransport( + new Options(), + $this->httpClient, + $this->payloadSerializer, + $logger + ); + + $result = $transport->send($event); + + $this->assertSame(ResultStatus::unknown(), $result->getStatus()); + } + /** * @group time-sensitive */ From 859b78907dd07097a80316eebf7141817ce7bfef Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Tue, 10 Oct 2023 10:56:48 +0200 Subject: [PATCH 12/20] CS --- src/HttpClient/Response.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/HttpClient/Response.php b/src/HttpClient/Response.php index 2c411dfb1..f2964aff5 100644 --- a/src/HttpClient/Response.php +++ b/src/HttpClient/Response.php @@ -12,17 +12,17 @@ final class Response /** * @var int The HTTP status code */ - protected $statusCode; + private $statusCode; /** * @var string[] The HTTP response headers */ - protected $headers; + private $headers; /** * @var string The cURL error and error message */ - protected $error; + private $error; /** * @param string[] $headers From e1c176b5bef4dd8380c22259ea4bab1aed84b5da Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Wed, 11 Oct 2023 01:10:30 +0200 Subject: [PATCH 13/20] Improve header parsing --- src/HttpClient/HttpClient.php | 66 ++---------------------- src/HttpClient/Response.php | 29 ++++++++--- src/Util/Http.php | 69 +++++++++++++++++++++++++ tests/Transport/HttpTransportTest.php | 2 +- tests/Transport/RateLimiterTest.php | 14 ++--- tests/Util/HttpTest.php | 74 +++++++++++++++++++++++++++ 6 files changed, 177 insertions(+), 77 deletions(-) create mode 100644 src/Util/Http.php create mode 100644 tests/Util/HttpTest.php diff --git a/src/HttpClient/HttpClient.php b/src/HttpClient/HttpClient.php index f9544c85b..868ea2b35 100644 --- a/src/HttpClient/HttpClient.php +++ b/src/HttpClient/HttpClient.php @@ -4,9 +4,8 @@ namespace Sentry\HttpClient; -use Sentry\Client; -use Sentry\Dsn; use Sentry\Options; +use Sentry\Util\Http; /** * @internal @@ -38,7 +37,7 @@ public function sendRequest(string $requestData, Options $options): Response $curlHandle = curl_init(); curl_setopt($curlHandle, \CURLOPT_URL, $dsn->getEnvelopeApiEndpointUrl()); - curl_setopt($curlHandle, \CURLOPT_HTTPHEADER, $this->getRequestHeaders($dsn)); + curl_setopt($curlHandle, \CURLOPT_HTTPHEADER, Http::getRequestHeaders($dsn, $this->sdkIdentifier, $this->sdkVersion)); curl_setopt($curlHandle, \CURLOPT_USERAGENT, $this->sdkIdentifier . '/' . $this->sdkVersion); curl_setopt($curlHandle, \CURLOPT_TIMEOUT, $options->getHttpTimeout()); curl_setopt($curlHandle, \CURLOPT_CONNECTTIMEOUT, $options->getHttpConnectTimeout()); @@ -47,11 +46,6 @@ public function sendRequest(string $requestData, Options $options): Response curl_setopt($curlHandle, \CURLOPT_POSTFIELDS, $requestData); curl_setopt($curlHandle, \CURLOPT_RETURNTRANSFER, true); curl_setopt($curlHandle, \CURLOPT_HEADER, true); - /** - * If we add support for CURL_HTTP_VERSION_2_0, we need - * case-insensitive header handling, as HTTP 2.0 headers - * are all lowercase. - */ curl_setopt($curlHandle, \CURLOPT_HTTP_VERSION, \CURL_HTTP_VERSION_1_1); $httpSslVerifyPeer = $options->getHttpSslVerifyPeer(); @@ -86,64 +80,10 @@ public function sendRequest(string $requestData, Options $options): Response $statusCode = curl_getinfo($curlHandle, \CURLINFO_HTTP_CODE); $headerSize = curl_getinfo($curlHandle, \CURLINFO_HEADER_SIZE); - $headers = $this->getResponseHeaders($headerSize, (string) $body); + $headers = Http::getResponseHeaders($headerSize, (string) $body); curl_close($curlHandle); return new Response($statusCode, $headers, ''); } - - /** - * @return string[] - */ - protected function getRequestHeaders(Dsn $dsn): array - { - $headers = [ - 'Content-Type' => 'application/x-sentry-envelope', - ]; - - $data = [ - 'sentry_version' => Client::PROTOCOL_VERSION, - 'sentry_client' => $this->sdkIdentifier . '/' . $this->sdkVersion, - 'sentry_key' => $dsn->getPublicKey(), - ]; - - if (null !== $dsn->getSecretKey()) { - $data['sentry_secret'] = $dsn->getSecretKey(); - } - - $authHeader = []; - foreach ($data as $headerKey => $headerValue) { - $authHeader[] = $headerKey . '=' . $headerValue; - } - - return array_merge($headers, [ - 'X-Sentry-Auth' => 'Sentry ' . implode(', ', $authHeader), - ]); - } - - /** - * @TODO(michi) This might need a bit more love, - * but we only really care about X-Sentry-Rate-Limits and Retry-After - * - * @return string[] - */ - protected function getResponseHeaders(?int $headerSize, string $body): array - { - $headers = []; - $rawHeaders = explode("\r\n", trim(substr($body, 0, $headerSize))); - - foreach ($rawHeaders as $value) { - if (!str_contains($value, ':')) { - continue; - } - [$name, $value] = explode(':', $value, 2); - $value = trim($value); - $name = trim($name); - - $headers[$name] = $value; - } - - return $headers; - } } diff --git a/src/HttpClient/Response.php b/src/HttpClient/Response.php index f2964aff5..e7afa970b 100644 --- a/src/HttpClient/Response.php +++ b/src/HttpClient/Response.php @@ -15,7 +15,7 @@ final class Response private $statusCode; /** - * @var string[] The HTTP response headers + * @var string[][] */ private $headers; @@ -25,7 +25,7 @@ final class Response private $error; /** - * @param string[] $headers + * @param string[][] $headers */ public function __construct(int $statusCode, array $headers, string $error) { @@ -44,14 +44,31 @@ public function isSuccess(): bool return $this->statusCode >= 200 && $this->statusCode <= 299; } - public function hasHeader(string $headerName): bool + public function hasHeader(string $name): bool { - return \array_key_exists($headerName, $this->headers); + return \array_key_exists($name, $this->headers); } - public function getHeaderLine(string $headerName): string + /** + * @return string[] + */ + public function getHeader(string $header): array { - return $this->headers[$headerName] ?? ''; + if (!$this->hasHeader($header)) { + return []; + } + + return $this->headers[$header]; + } + + public function getHeaderLine(string $name): string + { + $value = $this->getHeader($name); + if (empty($value)) { + return ''; + } + + return implode(',', $value); } public function getError(): string diff --git a/src/Util/Http.php b/src/Util/Http.php new file mode 100644 index 000000000..c2aaf026c --- /dev/null +++ b/src/Util/Http.php @@ -0,0 +1,69 @@ + 'application/x-sentry-envelope', + ]; + + $data = [ + 'sentry_version' => Client::PROTOCOL_VERSION, + 'sentry_client' => $sdkIdentifier . '/' . $sdkVersion, + 'sentry_key' => $dsn->getPublicKey(), + ]; + + if (null !== $dsn->getSecretKey()) { + $data['sentry_secret'] = $dsn->getSecretKey(); + } + + $authHeader = []; + foreach ($data as $headerKey => $headerValue) { + $authHeader[] = $headerKey . '=' . $headerValue; + } + + return array_merge($headers, [ + 'X-Sentry-Auth' => 'Sentry ' . implode(', ', $authHeader), + ]); + } + + /** + * @return string[][] + */ + public static function getResponseHeaders(?int $headerSize, string $body): array + { + $headers = []; + $rawHeaders = explode("\r\n", trim(substr($body, 0, $headerSize))); + + foreach ($rawHeaders as $value) { + if (!str_contains($value, ':')) { + continue; + } + [$name, $value] = explode(':', $value, 2); + $value = trim($value); + $name = trim($name); + + if (isset($headers[$name])) { + $headers[$name][] = $value; + } else { + $headers[$name] = (array) $value; + } + } + + return $headers; + } +} diff --git a/tests/Transport/HttpTransportTest.php b/tests/Transport/HttpTransportTest.php index 077dc6865..6efa78e42 100644 --- a/tests/Transport/HttpTransportTest.php +++ b/tests/Transport/HttpTransportTest.php @@ -179,7 +179,7 @@ public function testSendFailsDueToExceedingRateLimits(): void $this->httpClient->expects($this->once()) ->method('sendRequest') - ->willReturn(new Response(429, ['Retry-After' => '60'], '')); + ->willReturn(new Response(429, ['Retry-After' => ['60']], '')); $transport = new HttpTransport( new Options(), diff --git a/tests/Transport/RateLimiterTest.php b/tests/Transport/RateLimiterTest.php index 106a7c54d..14315d401 100644 --- a/tests/Transport/RateLimiterTest.php +++ b/tests/Transport/RateLimiterTest.php @@ -60,31 +60,31 @@ public static function handleResponseDataProvider(): \Generator yield 'Back-off using X-Sentry-Rate-Limits header with single category' => [ Event::createEvent(), - new Response(429, ['X-Sentry-Rate-Limits' => '60:error:org'], ''), + new Response(429, ['X-Sentry-Rate-Limits' => ['60:error:org']], ''), 429, ]; yield 'Back-off using X-Sentry-Rate-Limits header with multiple categories' => [ Event::createEvent(), - new Response(429, ['X-Sentry-Rate-Limits' => '60:error;transaction:org'], ''), + new Response(429, ['X-Sentry-Rate-Limits' => ['60:error;transaction:org']], ''), 429, ]; yield 'Back-off using X-Sentry-Rate-Limits header with missing categories should lock them all' => [ Event::createEvent(), - new Response(429, ['X-Sentry-Rate-Limits' => '60::org'], ''), + new Response(429, ['X-Sentry-Rate-Limits' => ['60::org']], ''), 429, ]; yield 'Back-off using Retry-After header with number-based value' => [ Event::createEvent(), - new Response(429, ['Retry-After' => '60'], ''), + new Response(429, ['Retry-After' => ['60']], ''), 429, ]; yield 'Back-off using Retry-After header with date-based value' => [ Event::createEvent(), - new Response(429, ['Retry-After' => 'Sun, 02 February 2022 00:01:00 GMT'], ''), + new Response(429, ['Retry-After' => ['Sun, 02 February 2022 00:01:00 GMT']], ''), 429, ]; } @@ -99,7 +99,7 @@ public function testIsRateLimited(): void // Events should be rate-limited for 60 seconds, but transactions should // still be allowed to be sent - $this->rateLimiter->handleResponse(Event::createEvent(), new Response(429, ['X-Sentry-Rate-Limits' => '60:error:org'], '')); + $this->rateLimiter->handleResponse(Event::createEvent(), new Response(429, ['X-Sentry-Rate-Limits' => ['60:error:org']], '')); $this->assertTrue($this->rateLimiter->isRateLimited(EventType::event())); $this->assertFalse($this->rateLimiter->isRateLimited(EventType::transaction())); @@ -112,7 +112,7 @@ public function testIsRateLimited(): void // Both events and transactions should be rate-limited if all categories // are - $this->rateLimiter->handleResponse(Event::createTransaction(), new Response(429, ['X-Sentry-Rate-Limits' => '60:all:org'], '')); + $this->rateLimiter->handleResponse(Event::createTransaction(), new Response(429, ['X-Sentry-Rate-Limits' => ['60:all:org']], '')); $this->assertTrue($this->rateLimiter->isRateLimited(EventType::event())); $this->assertTrue($this->rateLimiter->isRateLimited(EventType::transaction())); diff --git a/tests/Util/HttpTest.php b/tests/Util/HttpTest.php new file mode 100644 index 000000000..109fcea96 --- /dev/null +++ b/tests/Util/HttpTest.php @@ -0,0 +1,74 @@ +assertSame($expectedResult, Http::getRequestHeaders($dsn, $sdkIdentifier, $sdkVersion)); + } + + public static function getRequestHeadersDataProvider(): \Generator + { + yield [ + Dsn::createFromString('http://public@example.com/1'), + 'sentry.sdk.identifier', + '1.2.3', + [ + 'Content-Type' => 'application/x-sentry-envelope', + 'X-Sentry-Auth' => 'Sentry sentry_version=7, sentry_client=sentry.sdk.identifier/1.2.3, sentry_key=public', + ], + ]; + + yield [ + Dsn::createFromString('http://public:secret@example.com/1'), + 'sentry.sdk.identifier', + '1.2.3', + [ + 'Content-Type' => 'application/x-sentry-envelope', + 'X-Sentry-Auth' => 'Sentry sentry_version=7, sentry_client=sentry.sdk.identifier/1.2.3, sentry_key=public, sentry_secret=secret', + ], + ]; + } + + /** + * @dataProvider getResponseHeadersDataProvider + */ + public function testGetResponseHeaders(?int $headerSize, string $body, array $expectedResult): void + { + $this->assertSame($expectedResult, Http::getResponseHeaders($headerSize, $body)); + } + + public static function getResponseHeadersDataProvider(): \Generator + { + yield [ + 128, + << ['nginx'], + 'Date' => ['Tue, 10 Oct 2023 10:00:00 GMT'], + 'Content-Type' => ['application/json'], + 'Content-Length' => ['41'], + ], + ]; + } +} From 40be49efe0a92bf715e5ad9a4c51288455cbdaa9 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Wed, 11 Oct 2023 01:38:27 +0200 Subject: [PATCH 14/20] Add more ClientBuilder tests --- tests/ClientBuilderTest.php | 63 +++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/tests/ClientBuilderTest.php b/tests/ClientBuilderTest.php index bd514bc24..552310d25 100644 --- a/tests/ClientBuilderTest.php +++ b/tests/ClientBuilderTest.php @@ -8,8 +8,15 @@ use Sentry\Client; use Sentry\ClientBuilder; use Sentry\Event; +use Sentry\HttpClient\HttpClient; +use Sentry\HttpClient\HttpClientInterface; +use Sentry\HttpClient\Response; use Sentry\Integration\IntegrationInterface; use Sentry\Options; +use Sentry\Transport\HttpTransport; +use Sentry\Transport\Result; +use Sentry\Transport\ResultStatus; +use Sentry\Transport\TransportInterface; final class ClientBuilderTest extends TestCase { @@ -70,6 +77,41 @@ public function testCreateWithNoOptionsIsTheSameAsDefaultOptions(): void ClientBuilder::create([]) ); } + + public function testDefaultHttpClientAndTransport() + { + $options = new Options(); + $clientBuilder = new ClientBuilder($options); + + $this->assertInstanceOf(HttpClient::class, $clientBuilder->getHttpClient()); + $this->assertInstanceOf(HttpTransport::class, $clientBuilder->getTransport()); + } + + public function testSettingCustomHttpClinet() + { + $httpClient = new CustomHttpClient(); + + $options = new Options([ + 'http_client' => $httpClient, + ]); + $clientBuilder = new ClientBuilder($options); + + $this->assertSame($httpClient, $clientBuilder->getHttpClient()); + $this->assertInstanceOf(HttpTransport::class, $clientBuilder->getTransport()); + } + + public function testSettingCustomTransport() + { + $transport = new CustomTransport(); + + $options = new Options([ + 'transport' => $transport, + ]); + $clientBuilder = new ClientBuilder($options); + + $this->assertInstanceOf(HttpClient::class, $clientBuilder->getHttpClient()); + $this->assertSame($transport, $clientBuilder->getTransport()); + } } final class StubIntegration implements IntegrationInterface @@ -78,3 +120,24 @@ public function setupOnce(): void { } } + +final class CustomHttpClient implements HttpClientInterface +{ + public function sendRequest(string $requestData, Options $options): Response + { + return new Response(0, [], ''); + } +} + +final class CustomTransport implements TransportInterface +{ + public function send(Event $event): Result + { + return new Result(ResultStatus::success()); + } + + public function close(?int $timeout = null): Result + { + return new Result(ResultStatus::success()); + } +} From a4ab6e04badf9925d6f7134e290a4fe5e2ddf720 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Wed, 11 Oct 2023 03:06:23 +0200 Subject: [PATCH 15/20] Add case-insensitive header handling --- src/HttpClient/Response.php | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/HttpClient/Response.php b/src/HttpClient/Response.php index e7afa970b..56d921c07 100644 --- a/src/HttpClient/Response.php +++ b/src/HttpClient/Response.php @@ -14,6 +14,11 @@ final class Response */ private $statusCode; + /** + * @var string[] + */ + private $headerNames = []; + /** * @var string[][] */ @@ -32,6 +37,10 @@ public function __construct(int $statusCode, array $headers, string $error) $this->statusCode = $statusCode; $this->headers = $headers; $this->error = $error; + + foreach ($headers as $name => $value) { + $this->headerNames[strtolower($name)] = $name; + } } public function getStatusCode(): int @@ -46,7 +55,7 @@ public function isSuccess(): bool public function hasHeader(string $name): bool { - return \array_key_exists($name, $this->headers); + return isset($this->headerNames[strtolower($name)]); } /** @@ -58,6 +67,8 @@ public function getHeader(string $header): array return []; } + $header = $this->headerNames[strtolower($header)]; + return $this->headers[$header]; } From 6731f382614fd8027590638023d00ec8980161b7 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Wed, 18 Oct 2023 00:40:28 +0200 Subject: [PATCH 16/20] Refactor Http::getRequestHeaders --- src/Util/Http.php | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/Util/Http.php b/src/Util/Http.php index c2aaf026c..5ba85d94f 100644 --- a/src/Util/Http.php +++ b/src/Util/Http.php @@ -17,10 +17,6 @@ final class Http */ public static function getRequestHeaders(Dsn $dsn, string $sdkIdentifier, string $sdkVersion): array { - $headers = [ - 'Content-Type' => 'application/x-sentry-envelope', - ]; - $data = [ 'sentry_version' => Client::PROTOCOL_VERSION, 'sentry_client' => $sdkIdentifier . '/' . $sdkVersion, @@ -36,9 +32,10 @@ public static function getRequestHeaders(Dsn $dsn, string $sdkIdentifier, string $authHeader[] = $headerKey . '=' . $headerValue; } - return array_merge($headers, [ + return [ + 'Content-Type' => 'application/x-sentry-envelope', 'X-Sentry-Auth' => 'Sentry ' . implode(', ', $authHeader), - ]); + ]; } /** From a2f55940b2e7e97b381720cf4997f7fd464f2937 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Wed, 18 Oct 2023 00:43:43 +0200 Subject: [PATCH 17/20] Add CURLOPT_HEADEROPT=CURLHEADER_SEPARATE for proxy connections --- src/HttpClient/HttpClient.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/HttpClient/HttpClient.php b/src/HttpClient/HttpClient.php index 868ea2b35..6291370a1 100644 --- a/src/HttpClient/HttpClient.php +++ b/src/HttpClient/HttpClient.php @@ -56,6 +56,7 @@ public function sendRequest(string $requestData, Options $options): Response $httpProxy = $options->getHttpProxy(); if (null !== $httpProxy) { curl_setopt($curlHandle, \CURLOPT_PROXY, $httpProxy); + curl_setopt($curlHandle, \CURLOPT_HEADEROPT, \CURLHEADER_SEPARATE); } $httpProxyAuthentication = $options->getHttpProxyAuthentication(); From c9b3a2da444767b2793c5994a228e92c1675d4c1 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Wed, 18 Oct 2023 00:58:25 +0200 Subject: [PATCH 18/20] Refactor response header parsing --- src/HttpClient/HttpClient.php | 13 +++++++++---- src/Util/Http.php | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/HttpClient/HttpClient.php b/src/HttpClient/HttpClient.php index 6291370a1..a8deb4405 100644 --- a/src/HttpClient/HttpClient.php +++ b/src/HttpClient/HttpClient.php @@ -36,6 +36,12 @@ public function sendRequest(string $requestData, Options $options): Response } $curlHandle = curl_init(); + + $responseHeaders = []; + $responseHeaderCallback = function ($curlHandle, $headerLine) use (&$responseHeaders) { + return Http::parseResponseHeaders($headerLine, $responseHeaders); + }; + curl_setopt($curlHandle, \CURLOPT_URL, $dsn->getEnvelopeApiEndpointUrl()); curl_setopt($curlHandle, \CURLOPT_HTTPHEADER, Http::getRequestHeaders($dsn, $this->sdkIdentifier, $this->sdkVersion)); curl_setopt($curlHandle, \CURLOPT_USERAGENT, $this->sdkIdentifier . '/' . $this->sdkVersion); @@ -45,7 +51,8 @@ public function sendRequest(string $requestData, Options $options): Response curl_setopt($curlHandle, \CURLOPT_POST, true); curl_setopt($curlHandle, \CURLOPT_POSTFIELDS, $requestData); curl_setopt($curlHandle, \CURLOPT_RETURNTRANSFER, true); - curl_setopt($curlHandle, \CURLOPT_HEADER, true); + // curl_setopt($curlHandle, \CURLOPT_HEADER, true); + curl_setopt($curlHandle, \CURLOPT_HEADERFUNCTION, $responseHeaderCallback); curl_setopt($curlHandle, \CURLOPT_HTTP_VERSION, \CURL_HTTP_VERSION_1_1); $httpSslVerifyPeer = $options->getHttpSslVerifyPeer(); @@ -80,11 +87,9 @@ public function sendRequest(string $requestData, Options $options): Response } $statusCode = curl_getinfo($curlHandle, \CURLINFO_HTTP_CODE); - $headerSize = curl_getinfo($curlHandle, \CURLINFO_HEADER_SIZE); - $headers = Http::getResponseHeaders($headerSize, (string) $body); curl_close($curlHandle); - return new Response($statusCode, $headers, ''); + return new Response($statusCode, $responseHeaders, ''); } } diff --git a/src/Util/Http.php b/src/Util/Http.php index 5ba85d94f..e9eb79b75 100644 --- a/src/Util/Http.php +++ b/src/Util/Http.php @@ -38,6 +38,21 @@ public static function getRequestHeaders(Dsn $dsn, string $sdkIdentifier, string ]; } + /** + * @param string[][] $headers + */ + public static function parseResponseHeaders(string $headerLine, &$headers): int + { + if (false === strpos($headerLine, ':')) { + return \strlen($headerLine); + } + + [$key, $value] = explode(':', trim($headerLine), 2); + $headers[trim($key)] = trim($value); + + return \strlen($headerLine); + } + /** * @return string[][] */ From cb00c86fa59b45869b015e7dc8376f186a018563 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Wed, 18 Oct 2023 01:03:18 +0200 Subject: [PATCH 19/20] CS --- src/HttpClient/HttpClient.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/HttpClient/HttpClient.php b/src/HttpClient/HttpClient.php index a8deb4405..17f6baa01 100644 --- a/src/HttpClient/HttpClient.php +++ b/src/HttpClient/HttpClient.php @@ -38,7 +38,7 @@ public function sendRequest(string $requestData, Options $options): Response $curlHandle = curl_init(); $responseHeaders = []; - $responseHeaderCallback = function ($curlHandle, $headerLine) use (&$responseHeaders) { + $responseHeaderCallback = function ($curlHandle, $headerLine) use (&$responseHeaders): int { return Http::parseResponseHeaders($headerLine, $responseHeaders); }; From 51a5b58b4af48a93304653dc44b2e5b73a9266b2 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Wed, 18 Oct 2023 16:49:28 +0200 Subject: [PATCH 20/20] Cleanup --- src/HttpClient/HttpClient.php | 1 - src/Util/Http.php | 26 -------------------------- tests/Util/HttpTest.php | 31 ------------------------------- 3 files changed, 58 deletions(-) diff --git a/src/HttpClient/HttpClient.php b/src/HttpClient/HttpClient.php index 17f6baa01..d75148757 100644 --- a/src/HttpClient/HttpClient.php +++ b/src/HttpClient/HttpClient.php @@ -51,7 +51,6 @@ public function sendRequest(string $requestData, Options $options): Response curl_setopt($curlHandle, \CURLOPT_POST, true); curl_setopt($curlHandle, \CURLOPT_POSTFIELDS, $requestData); curl_setopt($curlHandle, \CURLOPT_RETURNTRANSFER, true); - // curl_setopt($curlHandle, \CURLOPT_HEADER, true); curl_setopt($curlHandle, \CURLOPT_HEADERFUNCTION, $responseHeaderCallback); curl_setopt($curlHandle, \CURLOPT_HTTP_VERSION, \CURL_HTTP_VERSION_1_1); diff --git a/src/Util/Http.php b/src/Util/Http.php index e9eb79b75..a82152175 100644 --- a/src/Util/Http.php +++ b/src/Util/Http.php @@ -52,30 +52,4 @@ public static function parseResponseHeaders(string $headerLine, &$headers): int return \strlen($headerLine); } - - /** - * @return string[][] - */ - public static function getResponseHeaders(?int $headerSize, string $body): array - { - $headers = []; - $rawHeaders = explode("\r\n", trim(substr($body, 0, $headerSize))); - - foreach ($rawHeaders as $value) { - if (!str_contains($value, ':')) { - continue; - } - [$name, $value] = explode(':', $value, 2); - $value = trim($value); - $name = trim($name); - - if (isset($headers[$name])) { - $headers[$name][] = $value; - } else { - $headers[$name] = (array) $value; - } - } - - return $headers; - } } diff --git a/tests/Util/HttpTest.php b/tests/Util/HttpTest.php index 109fcea96..df00fd049 100644 --- a/tests/Util/HttpTest.php +++ b/tests/Util/HttpTest.php @@ -40,35 +40,4 @@ public static function getRequestHeadersDataProvider(): \Generator ], ]; } - - /** - * @dataProvider getResponseHeadersDataProvider - */ - public function testGetResponseHeaders(?int $headerSize, string $body, array $expectedResult): void - { - $this->assertSame($expectedResult, Http::getResponseHeaders($headerSize, $body)); - } - - public static function getResponseHeadersDataProvider(): \Generator - { - yield [ - 128, - << ['nginx'], - 'Date' => ['Tue, 10 Oct 2023 10:00:00 GMT'], - 'Content-Type' => ['application/json'], - 'Content-Length' => ['41'], - ], - ]; - } }