when bin/upgrade and bin/compile CLI error:Area code is not set

[Hayyan-Ebrahem]

Environment

• PHP Version: (5.6|7.0|7.1|7.4)
• Magento Version:2.4.4-p1
• Emico/tweakwise: v4.3.2

Steps to reproduce

. bin/magento setup:upgrade | bin/magento setup:di:compile;

Actual result
Area code is not set

In State.php line 153:

the cause of the error is in vendor/emico/tweakwise/Model/Config.php:100 AreaCode needs to be set

[ah-net]

@Hayyan-Ebrahem
I can fix this with an try and catch, but i'm not sure if this is the right solution. Because the AreaCode when running setup:upgrade or setup:di:compile should be set to "cronjob". If this is not the case, its likely that another module causes this and it may cause more problems for other modules that expect the areacode to be set.

I'll discuss this internally and see what the best solution is for this.

Can you copy the issue to the new repo? https://github.com/Tweakwise/Magento2Tweakwise/
Any fixes for this wil be added to the new repo.

[ah-net]

@Hayyan-Ebrahem We've discussed this internally.

We are not planning to fix this on our end. The reasons for this are:

1. The area code when running setup:upgrade or setup:di:compile should be set to 'cronjob'. If it's not, it's likely an indication that something is wrong in your environment. This may cause other (undetected) issues. It could be an problem with an module or an problem with an specific magento version. It has been an problem with magento 2.2.3 in the past.
   https://support.magento.com/hc/en-us/articles/360025918872--Area-code-is-not-set-error-when-running-setup-updgrade

2. If we fix this with an try and catch, then we can't detect an problem if the areacode is not set in the admin. This causes the up/crossell templates to not work on storeview level without throwing an error. Making debugging this in the future difficult.

If you think we should fix this problem on our end, please explain why. We are open to an solution that doesn't supress the error in the admin.

[jasperzeinstra]

I think this problem should be fixed inside the Tweakwise module. It's bad practice to check the area code in the constructor. I've already made a fix and it's very simple and very clean. The only place you need to know the store id is inside the method "getStoreConfig". There you want to retrieve the config based on the request param store id in the admin url. For all other request you just want this to be NULL. When it's NULL the method \Magento\Framework\App\Config\ScopeConfigInterface::getValue will look up the current store id.

So the best solution to fix this issue is to move the setting of the store id from the constructor to its own method. And then requesting that method inside "getStoreConfig" instead of directly looking at $this->storeId.

Here's the patch that you could apply to make that work.

Index: Model/Config.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/Model/Config.php b/Model/Config.php
--- a/Model/Config.php	
+++ b/Model/Config.php	(date 1662635254104)
@@ -14,6 +14,7 @@
 use Magento\Framework\App\Config\ScopeConfigInterface;
 use Magento\Framework\App\RequestInterface;
 use Magento\Framework\App\State;
+use Magento\Framework\Exception\LocalizedException;
 use Magento\Store\Model\ScopeInterface;
 use Magento\Store\Model\Store;
 use Magento\Framework\Serialize\Serializer\Json;
@@ -77,9 +78,14 @@
     protected $parsedFilterArguments;
 
     /**
-     * @var null|string
+     * @var RequestInterface
      */
-    protected $storeId = null;
+    protected $request;
+
+    /**
+     * @var State
+     */
+    protected $state;
 
     /**
      * Export constructor.
@@ -88,18 +94,27 @@
      * @param Json $jsonSerializer
      * @param RequestInterface $request
      * @param State $state
-     * @throws \Magento\Framework\Exception\LocalizedException
+     * @throws LocalizedException
      */
 
     public function __construct(ScopeConfigInterface $config, Json $jsonSerializer, RequestInterface $request, State $state)
     {
         $this->config = $config;
         $this->jsonSerializer = $jsonSerializer;
-
-        //only do this if its an admin request, to prevent setting the store id in the url in the frontend
-        if ($state->getAreaCode() === Area::AREA_ADMINHTML) {
-            $this->storeId = $request->getParam('store', null);
-        }
+        $this->request = $request;
+        $this->state = $state;
+    }
+
+    /**
+     * Only return store id from request param when area is Admin HTML
+     * On the frontend returning NULL is sufficient to keep using the current store id
+     *
+     * @return mixed|string|null
+     * @throws LocalizedException
+     */
+    public function getStoreId()
+    {
+        return ($this->state->getAreaCode() === Area::AREA_ADMINHTML) ? $this->request->getParam('store', null) : null;
     }
 
     /**
@@ -456,7 +471,7 @@
             return $store->getConfig($path);
         }
 
-        return $this->config->getValue($path, ScopeInterface::SCOPE_STORE, $this->storeId);
+        return $this->config->getValue($path, ScopeInterface::SCOPE_STORE, $this->getStoreId());
     }
 
     /**

Hopefully you'll agree on this. If you have question, feel free to ask them.

[ah-net]

@jasperzeinstra

Thank you for the feedback and fix.

If the problem can be fixed bij removing it from the constructor, than thats sounds like an excellent solution for this. I'll test this and implement the solution if everything works like it should.

[ah-net]

@jasperzeinstra @Hayyan-Ebrahem

Fixed in version 5.0.4 in the tweakwise/magento2tweakwise repo.
(https://github.com/Tweakwise/Magento2Tweakwise/releases/tag/v5.0.4)