From f47dcc2436a8314f87896174c83b330352c0dcf3 Mon Sep 17 00:00:00 2001 From: Matthias Vogel Date: Mon, 26 May 2025 10:28:21 +0200 Subject: [PATCH 1/2] =?UTF-8?q?=E2=AC=86=EF=B8=8F=20allow=20php=208.4=20+?= =?UTF-8?q?=20add=20rate-limiter=20to=20FlushCommand?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/tasks.yml | 6 +-- Classes/Command/FlushCommand.php | 37 ++++++++++++++++--- ...ontentObjectProductionExceptionHandler.php | 29 ++++++++------- Classes/Queue/FileQueue.php | 12 ++++++ Classes/Queue/QueueInterface.php | 2 + Classes/Service/ConfigService.php | 9 ++++- Classes/Service/ScopeConfig.php | 25 +++++++++---- Classes/Service/Sentry.php | 2 +- Configuration/Services.yaml | 4 ++ composer.json | 10 ++--- phpstan.neon | 2 +- 11 files changed, 100 insertions(+), 38 deletions(-) diff --git a/.github/workflows/tasks.yml b/.github/workflows/tasks.yml index 2374398..f7d3d5d 100644 --- a/.github/workflows/tasks.yml +++ b/.github/workflows/tasks.yml @@ -11,15 +11,15 @@ jobs: strategy: fail-fast: false matrix: - php: [ '8.1', '8.2', '8.3' ] + php: [ '8.1', '8.2', '8.3', '8.4' ] typo3: [ '11', '12' ] steps: - name: Setup PHP with PECL extension uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php }} - - uses: actions/checkout@v2 - - uses: actions/cache@v2 + - uses: actions/checkout@v4 + - uses: actions/cache@v4 with: path: ~/.composer/cache/files key: ${{ runner.os }}-${{ matrix.php }}-composer-${{ hashFiles('**/composer.lock') }} diff --git a/Classes/Command/FlushCommand.php b/Classes/Command/FlushCommand.php index 2355f89..45eb6f6 100644 --- a/Classes/Command/FlushCommand.php +++ b/Classes/Command/FlushCommand.php @@ -7,14 +7,15 @@ use Exception; use GuzzleHttp\Exception\ClientException; use GuzzleHttp\Exception\RequestException; -use Http\Client\HttpAsyncClient; -use Jean85\Exception\VersionMissingExceptionInterface; use Http\Client\Common\Exception\ClientErrorException; +use Http\Client\HttpAsyncClient; use Http\Discovery\Psr17FactoryDiscovery; +use Jean85\Exception\VersionMissingExceptionInterface; use Jean85\PrettyVersions; use Pluswerk\Sentry\Queue\Entry; use Pluswerk\Sentry\Queue\QueueInterface; use Pluswerk\Sentry\Service\Sentry; +use Psr\Http\Message\ResponseInterface; use Sentry\Client; use Sentry\Dsn; use Sentry\HttpClient\HttpClientFactory; @@ -25,6 +26,10 @@ use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; +use function assert; +use function sprintf; +use function usleep; + class FlushCommand extends Command { private HttpClientFactoryInterface $httpClientFactory; @@ -45,6 +50,7 @@ protected function configure(): void { parent::configure(); $this->addOption('limit-items', null, InputOption::VALUE_REQUIRED, 'How much queue entries should be processed', 60); + $this->addOption('req-per-sec', null, InputOption::VALUE_REQUIRED, 'How many requests per second should be sent', 5); } /** @@ -84,9 +90,17 @@ protected function execute(InputInterface $input, OutputInterface $output): int $requestFactory = Psr17FactoryDiscovery::findRequestFactory(); $sentryClient = Sentry::getInstance()->getClient(); - $i = (int)$input->getOption('limit-items'); + $option = $input->getOption('limit-items'); + assert(is_string($option) || is_int($option)); + $reqPerSec = $input->getOption('req-per-sec'); + assert(is_string($reqPerSec) || is_int($reqPerSec)); + $reqPerSec = (int)$reqPerSec; + $i = (int)$option; + $option = (int)$option; $output->writeln(sprintf('running with limit-items=%d', $i), $output::VERBOSITY_VERBOSE); + $output->writeln(sprintf('to do: %d queued entries', $this->queue->count() ?? -1), $output::VERBOSITY_VERBOSE); + $lastTime = microtime(true); do { $entry = $this->queue->pop(); if (!$entry instanceof Entry) { @@ -94,7 +108,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int } $i--; - $itemIndex = $input->getOption('limit-items') - $i; + $itemIndex = $option - $i; $output->writeln(sprintf('start with entry %d', $itemIndex), $output::VERBOSITY_VERBOSE); $dsn = Dsn::createFromString($entry->getDsn()); @@ -112,15 +126,28 @@ protected function execute(InputInterface $input, OutputInterface $output): int try { $response = $client->sendAsyncRequest($request)->wait(); // fallback for then sendRequest is not throwing ClientErrorException - if ($response->getStatusCode() >= 400) { + if ($response instanceof ResponseInterface && $response->getStatusCode() >= 400) { throw RequestException::create($request, $response); } } catch (ClientException | ClientErrorException $clientErrorException) { $output->writeln(sprintf('could not send to sentry: %s', $clientErrorException->getMessage()), $output::VERBOSITY_QUIET); $sentryClient && $sentryClient->captureException($clientErrorException); + if ($clientErrorException->getResponse()->getStatusCode() === 429) { + $output->writeln('Rate limit reached, waiting for sentry to recover sleep(1s)', $output::VERBOSITY_QUIET); + sleep(1); // wait for sentry to recover + } } $output->writeln(sprintf('done with at %d', $itemIndex), $output::VERBOSITY_VERBOSE); + if ($i % $reqPerSec === 0) { + $toSleep = max(0, (int)(1_000_000 - (microtime(true) - $lastTime) * 1_000_000)); + if ($toSleep) { + $output->writeln(sprintf('%d req/s (sleep %dms)', $reqPerSec, $toSleep / 1_000), $output::VERBOSITY_VERBOSE); + usleep($toSleep); + } + + $lastTime = microtime(true); + } } while ($i > 0); $output->writeln('done', $output::VERBOSITY_VERBOSE); diff --git a/Classes/Handler/ContentObjectProductionExceptionHandler.php b/Classes/Handler/ContentObjectProductionExceptionHandler.php index c2eb180..be3b659 100644 --- a/Classes/Handler/ContentObjectProductionExceptionHandler.php +++ b/Classes/Handler/ContentObjectProductionExceptionHandler.php @@ -6,36 +6,31 @@ use Exception; use Pluswerk\Sentry\Service\ConfigService; +use Symfony\Component\DependencyInjection\Attribute\Autowire; use Throwable; +use TYPO3\CMS\Frontend\ContentObject\Exception\ExceptionHandlerInterface; use TYPO3\CMS\Frontend\ContentObject\Exception\ProductionExceptionHandler; use Pluswerk\Sentry\Service\Sentry; use Sentry\SentrySdk; use Sentry\State\Scope; use TYPO3\CMS\Frontend\ContentObject\AbstractContentObject; -use Psr\Log\LoggerInterface; -use TYPO3\CMS\Core\Context\Context; -use TYPO3\CMS\Core\Crypto\Random; -class ContentObjectProductionExceptionHandler extends ProductionExceptionHandler +class ContentObjectProductionExceptionHandler implements ExceptionHandlerInterface { public function __construct( - Context $context, - Random $random, - LoggerInterface $logger, + protected ProductionExceptionHandler $productionExceptionHandler, protected ConfigService $configService, ) { - parent::__construct($context, $random, $logger); } /** - * @param AbstractContentObject|null $contentObject * @param array $contentObjectConfiguration * @throws Exception */ - public function handle(Exception $exception, AbstractContentObject $contentObject = null, $contentObjectConfiguration = []): string + public function handle(Exception $exception, ?AbstractContentObject $contentObject = null, $contentObjectConfiguration = []): string { // if parent class rethrows the exception the ProductionExceptionHandler will handle the Exception - $result = parent::handle($exception, $contentObject, $contentObjectConfiguration); + $result = $this->productionExceptionHandler->handle($exception, $contentObject, $contentObjectConfiguration); $oopsCode = $this->getOopsCodeFromResult($result); try { @@ -47,13 +42,13 @@ public function handle(Exception $exception, AbstractContentObject $contentObjec return $result . $this->getLink($oopsCode); } - public function getOopsCodeFromResult(string $result): string + private function getOopsCodeFromResult(string $result): string { $explode = explode(' ', $result); return $explode[array_key_last($explode)]; } - public function getLink(string $oopsCode): string + private function getLink(string $oopsCode): string { $dsn = SentrySdk::getCurrentHub()->getClient()?->getOptions()->getDsn(); if (!$dsn) { @@ -67,4 +62,12 @@ public function getLink(string $oopsCode): string $url = $schema . '://' . $host . '/organizations/' . $organizationName . '/issues/?project=' . $projectId . '&query=oops_code%3A' . $oopsCode; return ' '; } + + /** + * @param array $configuration + */ + public function setConfiguration(array $configuration): void + { + $this->productionExceptionHandler->setConfiguration($configuration); + } } diff --git a/Classes/Queue/FileQueue.php b/Classes/Queue/FileQueue.php index a3734b0..42ac760 100644 --- a/Classes/Queue/FileQueue.php +++ b/Classes/Queue/FileQueue.php @@ -11,6 +11,8 @@ use TYPO3\CMS\Core\Core\Environment; use TYPO3\CMS\Core\Utility\GeneralUtility; +use function is_array; + class FileQueue implements QueueInterface { private string $directory; @@ -26,6 +28,12 @@ public function __construct(private int $limit = 10000, private bool $compress = } } + public function count(): int + { + $iterator = new FilesystemIterator($this->directory, FilesystemIterator::SKIP_DOTS); + return iterator_count($iterator); + } + /** * @throws JsonException */ @@ -73,6 +81,10 @@ public function pop(): ?Entry return null; } + if (!is_array($data)) { + return null; + } + if (!isset($data['dsn'], $data['isEnvelope'], $data['payload'])) { return null; } diff --git a/Classes/Queue/QueueInterface.php b/Classes/Queue/QueueInterface.php index 12104dc..3b4c398 100644 --- a/Classes/Queue/QueueInterface.php +++ b/Classes/Queue/QueueInterface.php @@ -6,6 +6,8 @@ interface QueueInterface { + public function count(): ?int; + public function pop(): ?Entry; public function push(Entry $entry): void; diff --git a/Classes/Service/ConfigService.php b/Classes/Service/ConfigService.php index 0b2c47b..f3a8f2a 100644 --- a/Classes/Service/ConfigService.php +++ b/Classes/Service/ConfigService.php @@ -31,7 +31,12 @@ private function getEnv(string $env): ?string private function getConfig(string $path): ?string { try { - return $this->configuration->get('sentry', $path) ?: null; + $config = $this->configuration->get('sentry', $path); + if (!is_string($config)) { + return null; + } + + return $config ?: null; } catch (ExtensionConfigurationPathDoesNotExistException) { return null; } @@ -60,7 +65,7 @@ public function isEnabled(): bool return !$this->isDisabled(); } - public function getErrorsToReport(): ?int + public function getErrorsToReport(): int { return (int)( $this->getEnv('SENTRY_ERRORS_TO_REPORT') diff --git a/Classes/Service/ScopeConfig.php b/Classes/Service/ScopeConfig.php index fef6106..f632295 100644 --- a/Classes/Service/ScopeConfig.php +++ b/Classes/Service/ScopeConfig.php @@ -45,7 +45,7 @@ protected function getExtras(): array } /** - * @return array{username: string, id?: string, email?: string}|array{} + * @return array{username: string, id: non-falsy-string, email: non-falsy-string}|array{username: string, id: non-falsy-string}|array{username: string}|array{} */ protected function getUserContext(): array { @@ -64,16 +64,25 @@ protected function getUserContext(): array $userAuthentication = $GLOBALS['BE_USER'] ?? null; } + if (!$username || !is_string($username)) { + return []; + } + $user = []; - if ($username) { - $user['username'] = $username; - if ($userAuthentication instanceof AbstractUserAuthentication && is_array($userAuthentication->user)) { - $user['id'] = $userAuthentication->user_table . ':' . ($userAuthentication->user['uid'] ?? null); - $user['email'] = $userAuthentication->user['email'] ?? null; - } + $user['username'] = $username; + if (!$userAuthentication instanceof AbstractUserAuthentication || !is_array($userAuthentication->user)) { + return $user; + } + + $user['id'] = $userAuthentication->user_table . ':' . ($userAuthentication->user['uid'] ?? null); + + $email = $userAuthentication->user['email'] ?? null; + if (!$email) { + return $user; } - return array_filter($user); + $user['email'] = $email; + return $user; } protected function getApplicationType(): ?ApplicationType diff --git a/Classes/Service/Sentry.php b/Classes/Service/Sentry.php index 9f36357..f30519b 100644 --- a/Classes/Service/Sentry.php +++ b/Classes/Service/Sentry.php @@ -86,7 +86,7 @@ public function getHub(): ?HubInterface return SentrySdk::getCurrentHub(); } - public function withScope(Throwable $exception, callable $withScope = null): void + public function withScope(Throwable $exception, ?callable $withScope = null): void { $withScope ??= static fn(Scope $scope) => null; withScope( diff --git a/Configuration/Services.yaml b/Configuration/Services.yaml index 6a3349b..ed49ec6 100644 --- a/Configuration/Services.yaml +++ b/Configuration/Services.yaml @@ -37,7 +37,11 @@ services: command: 'pluswerk:sentry:flush' description: 'Transports potentially queued events' + pluswerk.sentry.original.contentObject.productionExceptionHandler: + class: TYPO3\CMS\Frontend\ContentObject\Exception\ProductionExceptionHandler TYPO3\CMS\Frontend\ContentObject\Exception\ProductionExceptionHandler: class: Pluswerk\Sentry\Handler\ContentObjectProductionExceptionHandler public: true shared: false + arguments: + $productionExceptionHandler: '@pluswerk.sentry.original.contentObject.productionExceptionHandler' diff --git a/composer.json b/composer.json index 94ede20..06eed55 100644 --- a/composer.json +++ b/composer.json @@ -10,7 +10,7 @@ "docs": "https://github.com/pluswerk/sentry/blob/master/README.md" }, "require": { - "php": "~8.1.0 || ~8.2.0 || ~8.3.0", + "php": "~8.1.0 || ~8.2.0 || ~8.3.0 || ~8.4.0", "ext-fileinfo": "*", "composer-runtime-api": "^2", "http-interop/http-factory-guzzle": "^1.0", @@ -20,10 +20,10 @@ "typo3/cms-frontend": "^11.5 || ^12.4" }, "require-dev": { - "pluswerk/grumphp-config": "^7", - "saschaegerer/phpstan-typo3": "^1.10.0", - "ssch/typo3-rector": "^2.4.0", - "symfony/http-client": "^5.4 || ^6.4" + "pluswerk/grumphp-config": "^7.2.0", + "saschaegerer/phpstan-typo3": "^1.10.2", + "ssch/typo3-rector": "^2.13.1", + "symfony/http-client": "^5.4 || ^6.4.19" }, "autoload": { "psr-4": { diff --git a/phpstan.neon b/phpstan.neon index e178ba6..c8d1069 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -3,5 +3,5 @@ includes: - vendor/andersundsehr/phpstan-git-files/extension.php parameters: - level: 8 + level: max reportUnmatchedIgnoredErrors: false From 382b2ef307942b18d3519fb964f25df805db981b Mon Sep 17 00:00:00 2001 From: Matthias Vogel Date: Mon, 26 May 2025 17:00:32 +0200 Subject: [PATCH 2/2] Apply suggestions from code review --- Classes/Command/FlushCommand.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Classes/Command/FlushCommand.php b/Classes/Command/FlushCommand.php index 45eb6f6..bfc40e9 100644 --- a/Classes/Command/FlushCommand.php +++ b/Classes/Command/FlushCommand.php @@ -133,8 +133,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int $output->writeln(sprintf('could not send to sentry: %s', $clientErrorException->getMessage()), $output::VERBOSITY_QUIET); $sentryClient && $sentryClient->captureException($clientErrorException); if ($clientErrorException->getResponse()->getStatusCode() === 429) { - $output->writeln('Rate limit reached, waiting for sentry to recover sleep(1s)', $output::VERBOSITY_QUIET); - sleep(1); // wait for sentry to recover + $output->writeln('Rate limit reached, waiting for sentry to recover sleep(5s)', $output::VERBOSITY_QUIET); + sleep(5); // wait for sentry to recover } }