search

baldwin-agency / magento2-module-url-data-integrity-checker

Magento 2 module which can find potential url related problems in your catalog data
MIT License
261 stars 28 forks source link

Update CheckProductUrlPaths.php #21

Closed senders closed 3 years ago

senders commented 3 years ago

Prevent "Area code is already set" Error.

hostep commented 3 years ago

Hi @senders

Can you give a little bit of context for this? How can we trigger this problem? Which Magento version? Any custom extensions trying to manipulate the area as well? I'm not running into that error on a clean Magento 2.4.2 installation over here ...

senders commented 3 years ago

I am getting it on a 2.4.2 installation when executing your cli commands. Of course some extensions are installed. My additional code is just checking if the areacode was set before. It is working fine for me after adding the if clause, for everyone else it should working like before. It is just a double check.

hostep commented 3 years ago

Okay, I'm still wondering where this is coming from.

I'm not opposed to the PR however, I don't think it can hurt.

Can you let me know one more thing, what does $this->appState->getAreaCode() return in your case? Which area is it?

hostep commented 3 years ago

Hi @senders

Can you do me a favor and try running bin/magento catalog:images:resize in the project where you have this problem and let me know if that also fails with the same error? When you don't get an error, you can abort the command with ctrl+c, it shouldn't cause any harm (but if you want to be certain, test it out on a local or staging environment).

The reason why I'm asking is because I'm almost convinced that we should not try to fix this problem in this module. I have this hunch that in one of the customisations done in your shop, there will be a constructor that also sets the area. If that is the case, then this is a bug in that particular module, preferably you shouldn't change state or execute logic in constructors of classes in Magento modules. That logic should be moved to the code that actually executes something, and shouldn't be in the constructor.

Thanks!

hostep commented 3 years ago

Ni @senders: would you already have found time to provide some feedback? That would be appreciated? Thanks! 🙂

hostep commented 3 years ago

Closing PR, since I haven't received any feedback in over a month and because I think we shouldn't fix issues that 3rd party's are causing by using bad code.