Paymentsense-DevSupport / magento2-paymentsense-module

Paymentsense Module for Magento 2 Open Source (Community Edition)
GNU General Public License v3.0
3 stars 2 forks source link

Incompatibility with Monolog\Logger::debug #8

Open ronnyadsetts opened 2 years ago

ronnyadsetts commented 2 years ago

We're seeing the following errors with a magento setup:di:compile:

PHP Fatal error: Declaration of Paymentsense\Payments\Helper\PaymentsenseLogger::debug($message, array $context = []) must be compatible with Monolog\Logger::debug($message, array $context = []): void in /home/ronny/public_html/atroposbooks.co.uk/htdocs/vendor/paymentsense/magento2-module/Helper/PaymentsenseLogger.php on line 118

MiBPHP Fatal error: Declaration of Paymentsense\Payments\Helper\PaymentsenseLogger::info($message, array $context = []) must be compatible with Monolog\Logger::info($message, array $context = []): void in /home/ronny/public_html/atroposbooks.co.uk/htdocs/vendor/paymentsense/magento2-module/Helper/PaymentsenseLogger.php on line 99

This diff fixes the issues:

--- vendor/paymentsense/magento2-module/Helper/PaymentsenseLogger.php.OLD      2022-06-10 16:05:09.804873843 +0100
+++ vendor/paymentsense/magento2-module/Helper/PaymentsenseLogger.php   2022-06-10 16:05:57.630060853 +0100
@@ -60,13 +60,11 @@
      * @param  array   $context The log context
      * @return bool Whether the record has been processed
      */
-    public function error($message, array $context = [])
+    public function error($message, array $context = []): void
     {
-        $result = false;
         if ($this->getLogLevel()>=1) {
-            $result = $this->addRecord(static::ERROR, $message, $context);
+            $this->addRecord(static::ERROR, $message, $context);
         }
-        return $result;
     }

     /**
@@ -78,13 +76,11 @@
      * @param  array   $context The log context
      * @return bool Whether the record has been processed
      */
-    public function warning($message, array $context = [])
+    public function warning($message, array $context = []):void
     {
-        $result = false;
         if ($this->getLogLevel()>=2) {
-            $result = $this->addRecord(static::WARNING, $message, $context);
+            $this->addRecord(static::WARNING, $message, $context);
         }
-        return $result;
     }

     /**
@@ -96,13 +92,11 @@
      * @param  array   $context The log context
      * @return bool Whether the record has been processed
      */
-    public function info($message, array $context = [])
+    public function info($message, array $context = []): void
     {
-        $result = false;
         if ($this->getLogLevel()>=3) {
-            $result = $this->addRecord(static::INFO, $message, $context);
+            $this->addRecord(static::INFO, $message, $context);
         }
-        return $result;
     }

     /**
@@ -115,8 +109,8 @@
      * @param  array   $context The log context
      * @return bool Whether the record has been processed
      */
-    public function debug($message, array $context = [])
+    public function debug($message, array $context = []): void
     {
-        return $this->addRecord(static::DEBUG, $message, $context);
+        $this->addRecord(static::DEBUG, $message, $context);
     }
 }
johnorourke commented 2 years ago

@ronnyadsetts might you have done a composer install that ignores dependencies? The return types were only introduced in psr/log 3.0.0 and most Magento modules (we're on v2.4.3-p2) explicitly require psr/log v1 or v2 - so our composer install selected monolog v1.27.0 which has no return types defined.

ronnyadsetts commented 2 years ago

@johnorourke I don't think I've ignored dependencies. I'm not a Magento guro by any means though so...

Anyway, this is Magento 2.4.4. Composer shows this for monolog:

$ composer why monolog/monolog magento/framework 103.0.4 requires monolog/monolog (^2.3) magento/magento2-base 2.4.4 requires monolog/monolog (^2.3) magento/magento2-functional-testing-framework 3.9.0 requires monolog/monolog (^2.3) magento/product-community-edition 2.4.4 requires monolog/monolog (^2.3) swissup/module-marketplace 1.9.7 requires monolog/monolog (*)

$ composer show monolog/monolog | grep ^version versions : * 2.7.0

git blame shows that the explicit return types were added for version 2.0.0:

https://github.com/Seldaek/monolog/commit/a5876bed1d0f49770acd35feceae3388480b6c55

johnorourke commented 2 years ago

@ronnyadsetts ahh, that explains it - we haven't tackled 2.4.4 yet! Hopefully the @PaymentsenseDevs will get on it then.

ronnyadsetts commented 2 years ago

@johnorourke OK, thanks. Hopefully there are no functionality issues with using Magento 2.4.4 and Paymentsense. This is a new install (replacing an old 1.9.* site) and 2.4.4 was the only Magento version that made sense from a support lifecycle point of view.