getFrontController causing 404

[rcurrington]

After upgrading from an older version to the current version, our store went into 404 mode where it would only produce 404's for every page. After digging in, it appears to be from the change where getFrontController was added to the session handler constructor.

$this->sessionHandler = new \Cm\RedisSession\Handler(
new Cm_RedisSession_Model_Session_Config(),
new Cm_RedisSession_Model_Session_Logger(),
Mage::app()->getFrontController()->getAction() && Mage::app()->getFrontController()->getAction()->getFlag('', self::FLAG_READ_ONLY) ?: false
);

Simply calling Mage::app()->getFrontController() causes the site to 404. We are using amasty fpc, and I suspect it has something to do with that.

[colinmollenhour]

That's one of the problems with the lazy-load pattern.. However I noticed that _initFrontController registers the controller in the registry so that could be used to check to see if the front controller exists.

Please try changing it to something like this and if it fixes it a PR would be appreciated:

$this->sessionHandler = new \Cm\RedisSession\Handler(
    new Cm_RedisSession_Model_Session_Config(),
    new Cm_RedisSession_Model_Session_Logger(),
    Mage::registry('controller') && Mage::app()->getFrontController()->getAction() && Mage::app()->getFrontController()->getAction()->getFlag('', self::FLAG_READ_ONLY) ?: false
);

[rcurrington]

I was going to create a pull request for this, but I am still seeing an issue related to this. Basically, when the session handler logs an exception, it’s complaining that we don’t have a store. I think this is due to the fact that if a page is cached, the store is not initiated, so the session handler doesn’t know about it. PHP Fatal error: Uncaught Mage_Core_Model_Store_Exception in app/code/core/Mage/Core/Model/App.php:1377 Stack trace: #0 app/code/core/Mage/Core/Model/App.php(848): Mage_Core_Model_App->throwStoreException() #1 app/Mage.php(353): Mage_Core_Model_App->getStore(NULL) #2 app/Mage.php(870): Mage::getStoreConfig('dev/log/excepti...') #3 app/code/community/Cm/RedisSession/Model/Session/Logger.php(59): Mage::logException(Object(Mage_Core_Model_Store_Exception)) #4 app/code/community/Cm/RedisSession/lib/src/Cm/RedisSession/Handler.php(650): Cm_RedisSession_Model_Session_Logger->logException(Object(Mage_Core_Model_Store_Exception)) #5 app/code/community/Cm/RedisSession/Model/Session.php(137): Cm\RedisSession\Handler->write('qr6fu6cf01t3clg...', '_secure_cookie_...') #6 [internal function]: Cm_RedisSession_Mod in app/code/core/Mage/Core/Model/App.php on line 1377 Not sure how to fix this one. Thanks,. Ryan

…

On Jun 28, 2019, at 10:50 AM, Colin Mollenhour ***@***.***> wrote: That's one of the problems with the lazy-load pattern.. However I noticed that _initFrontController registers the controller in the registry so that could be used to check to see if the front controller exists. Please try changing it to something like this and if it fixes it a PR would be appreciated: $this->sessionHandler = new \Cm\RedisSession\Handler( new Cm_RedisSession_Model_Session_Config(), new Cm_RedisSession_Model_Session_Logger(), Mage::registry('controller') && Mage::app()->getFrontController()->getAction() && Mage::app()->getFrontController()->getAction()->getFlag('', self::FLAG_READ_ONLY) ?: false ); — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#166?email_source=notifications&email_token=AAA2JNVAGBTCXOCSDXCEKX3P4YQK7A5CNFSM4H4F4HO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY2JH6Q#issuecomment-506762234>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAA2JNWYSEMPYNEJH47P743P4YQK7ANCNFSM4H4F4HOQ>.

[colinmollenhour]

Right, whatever code is trying to init the session, it should not be doing so unless the app is already inited. Maybe it worked before, but it seems like a bad thing to do either way.

[rcurrington]

Perhaps we modify the logger to not log an exception if getStore() fails?

…

-Ryan

On Jul 1, 2019, at 12:22 PM, Colin Mollenhour ***@***.***> wrote: Right, whatever code is trying to init the session, it should not be doing so unless the app is already inited. Maybe it worked before, but it seems like a bad thing to do either way. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#166?email_source=notifications&email_token=AAA2JNUCNO7YJOL7UZ6TK6DP5IVK7A5CNFSM4H4F4HO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY6UY5Y#issuecomment-507333751>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAA2JNQ7645MYCRT3KYFSUDP5IVK7ANCNFSM4H4F4HOQ>.

[colinmollenhour]

Perhaps we modify the logger to not log an exception if getStore() fails?

An exception should not be thrown if it is just to be ignored. I've pushed an update to not init the front controller.