From a5ac47eda8e1fc0183251431ed3123ce08f2aef4 Mon Sep 17 00:00:00 2001 From: indy koning Date: Mon, 7 Oct 2024 15:23:58 +0200 Subject: [PATCH 1/5] Raise PHPStan to level 5, fix phpcs, add labels and update workflows --- .github/PULL_REQUEST_TEMPLATE.md | 5 ++ .github/workflows/analyse.yml | 2 +- .github/workflows/php.yml | 28 +++++-- .../System/Config/DeploymentConfigInfo.php | 9 ++- Block/SentryScript.php | 33 +++++++- Controller/Adminhtml/Test/Sentry.php | 9 ++- Helper/Data.php | 80 +++++++++++++++++-- Helper/Version.php | 2 +- Model/Config/Source/LogLevel.php | 2 + Model/Config/Source/ScriptTagPlacement.php | 5 ++ Model/SentryInteraction.php | 14 ++++ Model/SentryLog.php | 25 ++++-- Plugin/GlobalExceptionCatcher.php | 8 ++ Plugin/LogrocketCustomerInfo.php | 26 ++++-- Plugin/MonologPlugin.php | 17 +++- README.md | 12 +++ composer.json | 17 +++- phpstan.neon | 4 +- .../templates/system/config/button.phtml | 9 ++- .../config/deployment-config-info.phtml | 16 ++-- view/frontend/templates/script/sentry.phtml | 27 ++++--- 21 files changed, 291 insertions(+), 59 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index d62725d..58684a8 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -18,3 +18,8 @@ Example: The exact commands you ran and their output, screenshots / videos if the pull request changes UI. --> + +**Checklist** + +- [ ] I've ran `composer run codestyle` +- [ ] I've ran `composer run phpstan` \ No newline at end of file diff --git a/.github/workflows/analyse.yml b/.github/workflows/analyse.yml index c687e18..abaefb3 100644 --- a/.github/workflows/analyse.yml +++ b/.github/workflows/analyse.yml @@ -22,4 +22,4 @@ jobs: run: composer install --no-interaction - name: Analyse - run: vendor/bin/phpstan analyse \ No newline at end of file + run: composer run analyse \ No newline at end of file diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index f08109c..9bc1dd0 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -1,9 +1,25 @@ -name: PHPCS -on: [push] +name: Code Style + +on: ['push', 'pull_request'] + jobs: - build: + test: runs-on: ubuntu-latest + name: PHPCS + steps: - - uses: actions/checkout@master - - name: PHPCS - run: docker run --rm -v $PWD:/code:ro domw/phpcs phpcs --colors --standard=Magento2 --report=full,summary,gitblame --extensions=php,phtml ./ + - name: Checkout code + uses: actions/checkout@v2 + + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: 8.2 + extensions: dom, curl, libxml, mbstring, zip, pdo, sqlite, pdo_sqlite, bcmath, soap, intl, gd, exif, iconv, imagick, fileinfo + coverage: none + + - name: Install dependencies + run: composer install --no-interaction + + - name: Analyse + run: composer run phpcs \ No newline at end of file diff --git a/Block/Adminhtml/System/Config/DeploymentConfigInfo.php b/Block/Adminhtml/System/Config/DeploymentConfigInfo.php index 17c546d..1a2a3fb 100644 --- a/Block/Adminhtml/System/Config/DeploymentConfigInfo.php +++ b/Block/Adminhtml/System/Config/DeploymentConfigInfo.php @@ -18,8 +18,8 @@ class DeploymentConfigInfo extends Field * DeploymentConfigInfo constructor. * * @param Context $context - * @param array $data * @param Version $version + * @param array $data */ public function __construct( Context $context, @@ -29,6 +29,13 @@ public function __construct( parent::__construct($context, $data); } + /** + * Render the field. + * + * @param AbstractElement $element + * + * @return string + */ public function render(AbstractElement $element) { return $this->_toHtml(); diff --git a/Block/SentryScript.php b/Block/SentryScript.php index 7325970..61c7e94 100644 --- a/Block/SentryScript.php +++ b/Block/SentryScript.php @@ -9,13 +9,15 @@ class SentryScript extends Template { - const CURRENT_VERSION = '8.7.0'; + public const CURRENT_VERSION = '8.7.0'; /** * SentryScript constructor. * * @param DataHelper $dataHelper + * @param Version $version * @param Template\Context $context + * @param Json $json * @param array $data */ public function __construct( @@ -92,26 +94,41 @@ public function getEnvironment() return $this->dataHelper->getEnvironment(); } + /** + * Whether to enable session replay. + */ public function useSessionReplay(): bool { return $this->dataHelper->useSessionReplay(); } + /** + * Get the session replay sample rate + */ public function getReplaySessionSampleRate(): float { return $this->dataHelper->getReplaySessionSampleRate(); } + /** + * Get the session replay error sample rate + */ public function getReplayErrorSampleRate(): float { return $this->dataHelper->getReplayErrorSampleRate(); } + /** + * Whether to block media during replay. + */ public function getReplayBlockMedia(): bool { return $this->dataHelper->getReplayBlockMedia(); } + /** + * Whether to show mask text. + */ public function getReplayMaskText(): bool { return $this->dataHelper->getReplayMaskText(); @@ -167,21 +184,35 @@ public function stripStoreCode() return $this->dataHelper->stripStoreCode(); } + /** + * Get Store code + * + * @return string + */ public function getStoreCode() { return $this->_storeManager->getStore()->getCode(); } + /** + * Whether tracing is enabled + */ public function isTracingEnabled(): bool { return $this->dataHelper->isTracingEnabled(); } + /** + * Get sample rate for tracing + */ public function getTracingSampleRate(): float { return $this->dataHelper->getTracingSampleRate(); } + /** + * Get a list of js errors to ignore. + */ public function getIgnoreJsErrors(): string { return $this->json->serialize($this->dataHelper->getIgnoreJsErrors()); diff --git a/Controller/Adminhtml/Test/Sentry.php b/Controller/Adminhtml/Test/Sentry.php index 3c5479a..80579a7 100755 --- a/Controller/Adminhtml/Test/Sentry.php +++ b/Controller/Adminhtml/Test/Sentry.php @@ -17,7 +17,7 @@ class Sentry extends Action * * @see _isAllowed() */ - const ADMIN_RESOURCE = 'JustBetter_Sentry::sentry'; + public const ADMIN_RESOURCE = 'JustBetter_Sentry::sentry'; /** * Sentry constructor. @@ -43,7 +43,7 @@ public function __construct( /** * Execute view action. * - * @return \Magento\Framework\Controller\ResultInterface + * @return \Magento\Framework\Controller\ResultInterface|\Magento\Framework\App\ResponseInterface */ public function execute() { @@ -68,7 +68,10 @@ public function execute() $result['content'] = implode(PHP_EOL, $activeWithReason['reasons']); } - return $this->getResponse()->representJson( + /** @var \Magento\Framework\App\Response\Http $response */ + $response = $this->getResponse(); + + return $response->representJson( $this->jsonSerializer->serialize($result) ); } diff --git a/Helper/Data.php b/Helper/Data.php index a28c4ac..29b87c1 100644 --- a/Helper/Data.php +++ b/Helper/Data.php @@ -21,8 +21,8 @@ class Data extends AbstractHelper { - const XML_PATH_SRS = 'sentry/general/'; - const XML_PATH_SRS_ISSUE_GROUPING = 'sentry/issue_grouping/'; + public const XML_PATH_SRS = 'sentry/general/'; + public const XML_PATH_SRS_ISSUE_GROUPING = 'sentry/issue_grouping/'; /** * @var ScopeConfigInterface @@ -77,6 +77,8 @@ public function __construct( } /** + * Get the sentry DSN. + * * @return mixed */ public function getDSN() @@ -84,22 +86,33 @@ public function getDSN() return $this->config['dsn']; } + /** + * Whether tracing is enabled. + */ public function isTracingEnabled(): bool { return $this->config['tracing_enabled'] ?? false; } + /** + * Get sample rate for tracing. + */ public function getTracingSampleRate(): float { - return (float) $this->config['tracing_sample_rate'] ?? 0.2; + return (float) ($this->config['tracing_sample_rate'] ?? 0.2); } + /** + * Get a list of integrations to disable. + */ public function getDisabledDefaultIntegrations(): array { return $this->config['disable_default_integrations'] ?? []; } /** + * Get list of js errors to ignore. + * * @return array|null */ public function getIgnoreJsErrors() @@ -116,7 +129,7 @@ public function getIgnoreJsErrors() : $this->serializer->unserialize($this->config['ignore_js_errors']); } catch (InvalidArgumentException $e) { throw new RuntimeException( - 'Sentry configuration error: `ignore_js_errors` has to be an array or `null`. Given type: '.gettype($list) + 'Sentry configuration error: `ignore_js_errors` has to be an array or `null`. Given type: '.gettype($list) // phpcs:ignore ); } @@ -124,6 +137,8 @@ public function getIgnoreJsErrors() } /** + * Get the sdk version string. + * * @return string the version of the js sdk of Sentry */ public function getJsSdkVersion(): string @@ -132,6 +147,8 @@ public function getJsSdkVersion(): string } /** + * Get the current environment. + * * @return mixed */ public function getEnvironment() @@ -140,6 +157,8 @@ public function getEnvironment() } /** + * Retrieve config values. + * * @param string $field * @param int|string|null $storeId * @@ -155,6 +174,8 @@ public function getConfigValue($field, $storeId = null) } /** + * Retrieve Sentry General config values. + * * @param string $code * @param null $storeId * @@ -166,6 +187,8 @@ public function getGeneralConfig($code, $storeId = null) } /** + * Gather all configuration. + * * @return array */ public function collectModuleConfig(): array @@ -194,6 +217,8 @@ public function collectModuleConfig(): array } /** + * Whether Sentry is active. + * * @return bool */ public function isActive(): bool @@ -202,6 +227,8 @@ public function isActive(): bool } /** + * Whether sentry is active, adding a reason why not. + * * @return array */ public function isActiveWithReason(): array @@ -229,6 +256,8 @@ public function isActiveWithReason(): array } /** + * Whether the application is in production. + * * @return bool */ public function isProductionMode(): bool @@ -237,6 +266,8 @@ public function isProductionMode(): bool } /** + * Get the current mode. + * * @return string */ public function getAppState(): string @@ -245,6 +276,8 @@ public function getAppState(): string } /** + * Is sentry enabled on development mode? + * * @return bool */ public function isOverwriteProductionMode(): bool @@ -267,10 +300,12 @@ public function getMagentoVersion(): string */ public function getStore() { - return $this->storeManager ? $this->storeManager->getStore() : null; + return $this->storeManager->getStore(); } /** + * Is php tracking enabled? + * * @return bool */ public function isPhpTrackingEnabled(): bool @@ -279,6 +314,8 @@ public function isPhpTrackingEnabled(): bool } /** + * Is the script tag enabled? + * * @return bool */ public function useScriptTag(): bool @@ -286,32 +323,49 @@ public function useScriptTag(): bool return $this->scopeConfig->isSetFlag(static::XML_PATH_SRS.'enable_script_tag'); } + /** + * Whether to enable session replay. + */ public function useSessionReplay(): bool { return $this->scopeConfig->isSetFlag(static::XML_PATH_SRS.'enable_session_replay'); } + /** + * Get the session replay sample rate + */ public function getReplaySessionSampleRate(): float { return $this->getConfigValue(static::XML_PATH_SRS.'replay_session_sample_rate') ?? 0.1; } + /** + * Get the session replay error sample rate + */ public function getReplayErrorSampleRate(): float { return $this->getConfigValue(static::XML_PATH_SRS.'replay_error_sample_rate') ?? 1; } + /** + * Whether to block media during replay. + */ public function getReplayBlockMedia(): bool { return $this->getConfigValue(static::XML_PATH_SRS.'replay_block_media') ?? true; } + /** + * Whether to show mask text. + */ public function getReplayMaskText(): bool { return $this->getConfigValue(static::XML_PATH_SRS.'replay_mask_text') ?? true; } /** + * Should we load the script tag in the current block? + * * @param string $blockName * * @return bool @@ -330,6 +384,8 @@ public function showScriptTagInThisBlock($blockName): bool } /** + * Get logrocket key. + * * @return mixed */ public function getLogrocketKey() @@ -338,6 +394,8 @@ public function getLogrocketKey() } /** + * Whether to use logrocket. + * * @return bool */ public function useLogrocket(): bool @@ -348,6 +406,8 @@ public function useLogrocket(): bool } /** + * Should logrocket identify. + * * @return bool */ public function useLogrocketIdentify(): bool @@ -358,6 +418,8 @@ public function useLogrocketIdentify(): bool } /** + * Whether to remove static content versioning from the url sent to sentry. + * * @return bool */ public function stripStaticContentVersion(): bool @@ -368,6 +430,8 @@ public function stripStaticContentVersion(): bool } /** + * Whether to remove store code from the url sent to sentry. + * * @return bool */ public function stripStoreCode(): bool @@ -378,6 +442,8 @@ public function stripStoreCode(): bool } /** + * Get the ErrorException reporting level we will send at. + * * @return int */ public function getErrorExceptionReporting(): int @@ -386,6 +452,8 @@ public function getErrorExceptionReporting(): int } /** + * Get a list of exceptions we should never send to Sentry. + * * @return array */ public function getIgnoreExceptions(): array @@ -402,6 +470,8 @@ public function getIgnoreExceptions(): array } /** + * Check whether we should capture the given exception based on severity and ignore exceptions. + * * @param Throwable $ex * * @return bool diff --git a/Helper/Version.php b/Helper/Version.php index 41923b7..b2cb3ba 100644 --- a/Helper/Version.php +++ b/Helper/Version.php @@ -78,7 +78,7 @@ protected function readValue($appMode) ); } $result = $this->generateVersion(); - $this->versionStorage->save($result); + $this->versionStorage->save((string) $result); } return $result; diff --git a/Model/Config/Source/LogLevel.php b/Model/Config/Source/LogLevel.php index ce01543..300a96f 100755 --- a/Model/Config/Source/LogLevel.php +++ b/Model/Config/Source/LogLevel.php @@ -8,6 +8,8 @@ class LogLevel implements ArrayInterface { /** + * Mapping of Monolog values to Strings + * * @return array */ public function toOptionArray() diff --git a/Model/Config/Source/ScriptTagPlacement.php b/Model/Config/Source/ScriptTagPlacement.php index 885109a..d44a38e 100755 --- a/Model/Config/Source/ScriptTagPlacement.php +++ b/Model/Config/Source/ScriptTagPlacement.php @@ -6,6 +6,11 @@ class ScriptTagPlacement implements ArrayInterface { + /** + * Mapping of script include positions to strings + * + * @return array + */ public function toOptionArray() { return [ diff --git a/Model/SentryInteraction.php b/Model/SentryInteraction.php index ea484f4..d151fdc 100644 --- a/Model/SentryInteraction.php +++ b/Model/SentryInteraction.php @@ -11,11 +11,25 @@ class SentryInteraction { + /** + * Initialize Sentry with passed config. + * + * @param array $config Sentry config @see: https://docs.sentry.io/platforms/php/configuration/ + * + * @return void + */ public function initialize($config) { init($config); } + /** + * Capture passed exception + * + * @param \Throwable $ex + * + * @return void + */ public function captureException(\Throwable $ex) { ob_start(); diff --git a/Model/SentryLog.php b/Model/SentryLog.php index 81cf9ea..e7108d6 100755 --- a/Model/SentryLog.php +++ b/Model/SentryLog.php @@ -22,10 +22,11 @@ class SentryLog extends Monolog * SentryLog constructor. * * @param string $name - * @param array $handlers - * @param array $processors * @param Data $data * @param Session $customerSession + * @param State $appState + * @param array $handlers + * @param array $processors */ public function __construct( $name, @@ -39,9 +40,11 @@ public function __construct( } /** - * @param $message - * @param $logLevel - * @param array $context + * Check and send log information to Sentry + * + * @param \Throwable|string $message + * @param int $logLevel + * @param array $context */ public function send($message, $logLevel, $context = []) { @@ -83,6 +86,11 @@ function (SentryScope $scope) use ($context, $customTags): void { } } + /** + * Attempt to add user information based on customerSession. + * + * @param SentryScope $scope + */ private function setUser(SentryScope $scope): void { try { @@ -103,6 +111,11 @@ private function setUser(SentryScope $scope): void } } + /** + * Check if we can retrieve customer data. + * + * @return bool + */ private function canGetCustomerData() { try { @@ -113,6 +126,8 @@ private function canGetCustomerData() } /** + * Add additional tags to the scope. + * * @param SentryScope $scope * @param array $customTags */ diff --git a/Plugin/GlobalExceptionCatcher.php b/Plugin/GlobalExceptionCatcher.php index c2b3cc4..7741230 100755 --- a/Plugin/GlobalExceptionCatcher.php +++ b/Plugin/GlobalExceptionCatcher.php @@ -33,6 +33,14 @@ public function __construct( ) { } + /** + * Wrap launch, start watching for exceptions. + * + * @param AppInterface $subject + * @param callable $proceed + * + * @return \Magento\Framework\App\ResponseInterface + */ public function aroundLaunch(AppInterface $subject, callable $proceed) { if ((!$this->sentryHelper->isActive()) || (!$this->sentryHelper->isPhpTrackingEnabled())) { diff --git a/Plugin/LogrocketCustomerInfo.php b/Plugin/LogrocketCustomerInfo.php index e7daeda..632eb35 100644 --- a/Plugin/LogrocketCustomerInfo.php +++ b/Plugin/LogrocketCustomerInfo.php @@ -8,20 +8,36 @@ class LogrocketCustomerInfo { + /** + * LogrocketCustomerInfo construct. + * + * @param CurrentCustomer $currentCustomer + * @param Session $customerSession + */ public function __construct( protected CurrentCustomer $currentCustomer, protected Session $customerSession ) { } + /** + * Add customer info to the section. + * + * @param Customer $subject + * @param array $result + * + * @return array $result + */ public function afterGetSectionData(Customer $subject, $result) { - if ($this->customerSession->isLoggedIn()) { - $customer = $this->currentCustomer->getCustomer(); - - $result['email'] = $customer->getEmail(); - $result['fullname'] = $customer->getFirstname().' '.$customer->getLastname(); + if (!$this->customerSession->isLoggedIn()) { + return $result; } + + $customer = $this->currentCustomer->getCustomer(); + + $result['email'] = $customer->getEmail(); + $result['fullname'] = $customer->getFirstname().' '.$customer->getLastname(); return $result; } diff --git a/Plugin/MonologPlugin.php b/Plugin/MonologPlugin.php index 416ed2b..651fbd4 100755 --- a/Plugin/MonologPlugin.php +++ b/Plugin/MonologPlugin.php @@ -11,7 +11,14 @@ class MonologPlugin extends Monolog { /** - * {@inheritdoc} + * @psalm-param array $processors + * + * @param string $name The logging channel, a simple descriptive name that is attached to all log records + * @param Data $sentryHelper + * @param SentryLog $sentryLog + * @param DeploymentConfig $deploymentConfig + * @param \Monolog\Handler\HandlerInterface[] $handlers Optional stack of handlers, the first one in the array is called first, etc. + * @param callable[] $processors Optional array of processors */ public function __construct( $name, @@ -27,9 +34,10 @@ public function __construct( /** * Adds a log record to Sentry. * - * @param int $level The logging level - * @param string $message The log message - * @param array $context The log context + * @param int $level The logging level + * @param string $message The log message + * @param array $context The log context + * @param DateTimeImmutable $datetime Datetime of log * * @return bool Whether the record has been processed */ @@ -43,6 +51,7 @@ public function addRecord( $this->sentryLog->send($message, $level, $context); } + // @phpstan-ignore argument.type return parent::addRecord($level, $message, $context, $datetime); } } diff --git a/README.md b/README.md index ca0f661..3542156 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,7 @@ # Magento 2 Sentry Logger +[![Latest Version on Packagist](https://img.shields.io/packagist/v/justbetter/magento2-sentry.svg?style=flat-square)](https://packagist.org/packages/justbetter/magento2-sentry) +[![Total Downloads](https://img.shields.io/packagist/dt/justbetter/magento2-sentry.svg?style=flat-square)](https://packagist.org/packages/justbetter/magento2-sentry) +[![PHPStan passing](https://img.shields.io/github/actions/workflow/status/justbetter/magento2-sentry/analyse.yml?label=PHPStan&style=flat-square)](https://github.com/justbetter/magento2-sentry/actions/workflows/analyse.yml) This Magento 2 module integrates the [Sentry sdk](https://github.com/getsentry/sentry-php) into magento 2. Depending on the log level configured in the backend of magento 2, notifications and errors can be send to sentry. @@ -92,6 +95,15 @@ The module is tested on Magento version 2.4.x with sentry sdk version 3.x. Magen ## Ideas, bugs or suggestions? Please create a [issue](https://github.com/justbetter/magento2-sentry/issues) or a [pull request](https://github.com/justbetter/magento2-sentry/pulls). +## Contributing +Contributing? Awesome! Thank you for your help improving the module! + +- When making a PR please add a description what you've added, and if relevant why. +- To save time on codestyle feedback, please run + - `composer install` + - `composer run codestyle` + - `composer run phpstan` + ## About us We’re a innovative development agency from The Netherlands building awesome websites, webshops and web applications with Laravel and Magento. Check out our website [justbetter.nl](https://justbetter.nl) and our [open source projects](https://github.com/justbetter). diff --git a/composer.json b/composer.json index 656bf47..2b9dab7 100755 --- a/composer.json +++ b/composer.json @@ -39,11 +39,22 @@ "config": { "allow-plugins": { "php-http/discovery": true, - "magento/composer-dependency-version-audit-plugin": true + "magento/composer-dependency-version-audit-plugin": true, + "dealerdirect/phpcodesniffer-composer-installer": true } }, + "scripts": { + "analyse": "vendor/bin/phpstan analyse --memory-limit='1G'", + "phpcs": "vendor/bin/phpcs --colors --standard=vendor/magento/magento-coding-standard/Magento2 --exclude=Generic.Files.LineLength --report=full,summary,gitblame --extensions=php,phtml --ignore=./vendor ./", + "phpcbf": "vendor/bin/phpcbf --colors --standard=vendor/magento/magento-coding-standard/Magento2 --exclude=Generic.Files.LineLength --extensions=php,phtml --ignore=./vendor ./ || exit 0", + "codestyle": [ + "@phpcbf", + "@phpcs" + ] + }, "require-dev": { - "bitexpert/phpstan-magento": "^0.11.0", - "phpstan/phpstan": "^1.10" + "bitexpert/phpstan-magento": "^0.32.0", + "magento/magento-coding-standard": "^34", + "phpstan/phpstan": "^1.12" } } diff --git a/phpstan.neon b/phpstan.neon index 0d4f657..3d73f2b 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -6,6 +6,4 @@ parameters: excludePaths: - vendor - Test/* - level: 1 - ignoreErrors: - - '#Magento\\Framework\\DataObjectFactory#' \ No newline at end of file + level: 5 \ No newline at end of file diff --git a/view/adminhtml/templates/system/config/button.phtml b/view/adminhtml/templates/system/config/button.phtml index 530bfc9..91a2a9e 100755 --- a/view/adminhtml/templates/system/config/button.phtml +++ b/view/adminhtml/templates/system/config/button.phtml @@ -1,11 +1,14 @@