Before we seriously step into Magento 2 era, please allow me to dissect one more Magento 1 bug. Couple of weeks ago we enabled compilation at one of our client's Magento EE installation as a part of holiday traffic preparations. Goal was to take advantage of performance boost provided by avoiding Magento autoloader through compiling all the Magento classes into several bulky PHP files. Unfortunately things didn't go to plan and we had to disable compilation couple of days later, due to bug in the way catalog product view controller handles 404 forwarding when product exists, but for some reason it isn't viewable (out of stock, disabled, etc.).
Issue report we received after enabling Magento compilation feature was that some of the products in our catalog display blank page. Quick inspection revealed that our server throws internal server error half way through page load, and that products in question exist but are currently manually disabled in Magento admin. To display product page or page not found for existing product, Magento must first load product data from database, in order to obtain information about it's status. If product is disabled or for some other reason not viewable, delegation to Mage_Cms_IndexController::noRouteAction()
is used. Here's the excerpt from Mage_Catalog_ProductController
:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 | <?php // app/code/core/Mage/Catalog/controllers/ProductController.php /** * Product controller * * @category Mage * @package Mage_Catalog */ class Mage_Catalog_ProductController extends Mage_Core_Controller_Front_Action { // Other code /** * Product view action */ public function viewAction() { // Get initial data from request $categoryId = (int) $this->getRequest()->getParam('category', false); $productId = (int) $this->getRequest()->getParam('id'); $specifyOptions = $this->getRequest()->getParam('options'); // Prepare helper and params $viewHelper = Mage::helper('catalog/product_view'); $params = new Varien_Object(); $params->setCategoryId($categoryId); $params->setSpecifyOptions($specifyOptions); // Render page try { $viewHelper->prepareAndRender($productId, $this, $params); } catch (Exception $e) { if ($e->getCode() == $viewHelper->ERR_NO_PRODUCT_LOADED) { if (isset($_GET['store']) && !$this->getResponse()->isRedirect()) { $this->_redirect(''); } elseif (!$this->getResponse()->isRedirect()) { $this->_forward('noRoute'); } } else { Mage::logException($e); $this->_forward('noRoute'); } } } // Other code } |
Line 41 of preceding code listing is where action takes place. What Mage_Core_Controller_Varien_Action::_forward()
does is set request action to noRoute
and a flag that catalog product view action was not dispatched successfully. Here's relevant code excerpt to illustrate:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 | <?php // app/code/core/Mage/Core/Controller/Varien/Action.php /** * Custom Zend_Controller_Action class (formally) * * Allows dispatching before and after events for each controller action * * @category Mage * @package Mage_Core * @author Magento Core Team <core@magentocommerce.com> */ abstract class Mage_Core_Controller_Varien_Action { // Other code /** * Throw control to different action (control and module if was specified). * * @param string $action * @param string|null $controller * @param string|null $module * @param array|null $params */ protected function _forward($action, $controller = null, $module = null, array $params = null) { $request = $this->getRequest(); $request->initForward(); if (isset($params)) { $request->setParams($params); } if (isset($controller)) { $request->setControllerName($controller); // Module should only be reset if controller has been specified if (isset($module)) { $request->setModuleName($module); } } $request->setActionName($action) ->setDispatched(false); } // Other code /** * Dispatch event before action * * @return void */ public function preDispatch() { // Other code Varien_Autoload::registerScope($this->getRequest()->getRouteName()); // Other code } // Other code } |
On preceding code listing, inside Mage_Core_Controller_Varien_Action::_forward()
you can observe delegation to another controller in action. By design, if action was not dispatched, Magento iterates over available routers one more time in order to find noRouterAction
, located inside Mage_Cms_IndexController
. For more information about Magento routing process, you can review one of my older posts or simply open your copy of Grokking Magento, Book 1 by Vinai Kopp. Root cause of our blank page issue is that by using delegation, Mage_Core_Controller_Varien_Action::preDispatch
gets fired twice, along with Varien_Autoload::registerScope($this->getRequest()->getRouteName());
displayed on preceding code listing. This is problematic due to implementation of Varien_Autoload::registerScope()
, here's relevant code excerpt:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 | <?php // lib/Varien/Autoload.php /** * Classes source autoload */ class Varien_Autoload { // Other code /** * Register autoload scope * This process allow include scope file which can contain classes * definition which are used for this scope * * @param string $code scope code */ static public function registerScope($code) { self::$_scope = $code; if (defined('COMPILER_INCLUDE_PATH')) { @include COMPILER_INCLUDE_PATH . DIRECTORY_SEPARATOR . self::SCOPE_FILE_PREFIX.$code.'.php'; } } // Other code } |
It's obvious that task of Varien_Autoload::registerScope()
is to include correct scope dependent compiled file if Magento compilation is enabled. This scope dependency is passed to this function as $code
variable, right from Mage_Core_Controller_Varien_Action::preDispatch
as $this->getRequest()->getRouteName()
. Route name actually reflects requested module, and by setting action to noRoute
requested route still remains catalog. So what actually happens is that Varien_Autoload::registerScope()
tries to include the same file twice using include
keyword, something that produces unrecoverable fatal error in PHP, all safely hidden under @ error suppression operator, which is primarily placed here to suppress PHP warning emitted if file to be included does not exist.
Fix for this issue is simple, either disable Magento compiler or pollute your local code pool with autoloader modified to use include_once
, instead of include
used by built in autoloader.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 | <?php // app/code/local/Varien/Autoload.php /** * Classes source autoload */ class Varien_Autoload { // Other code /** * Register autoload scope * This process allow include scope file which can contain classes * definition which are used for this scope * * @param string $code scope code */ static public function registerScope($code) { self::$_scope = $code; if (defined('COMPILER_INCLUDE_PATH')) { //@include COMPILER_INCLUDE_PATH . DIRECTORY_SEPARATOR . self::SCOPE_FILE_PREFIX.$code.'.php'; // Fix for internal server error being thrown at product page for disabled products @include_once COMPILER_INCLUDE_PATH . DIRECTORY_SEPARATOR . self::SCOPE_FILE_PREFIX.$code.'.php'; } } // Other code } |
Lesion of the day, no matter how mature our platform is, we'll never get rid of all the bugs.
Awesome explanation. Thanks for sharing it. Saved a lot of time for me in EE 1.4.1.0