Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use xdebug for error/exception handling #2741

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion index.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,4 @@
$bootstrap = \Magento\Framework\App\Bootstrap::create(BP, $_SERVER);
/** @var \Magento\Framework\App\Http $app */
$app = $bootstrap->createApplication('Magento\Framework\App\Http');
$bootstrap->run($app);
$bootstrap->run($app, false);
78 changes: 72 additions & 6 deletions lib/internal/Magento/Framework/App/Bootstrap.php
Original file line number Diff line number Diff line change
Expand Up @@ -244,29 +244,82 @@ public function createApplication($type, $arguments = [])
* Runs an application
*
* @param \Magento\Framework\AppInterface $application
* @param bool $alwaysHandleExceptions Whether to handle exceptions regardless of developer mode setting
* @return void
*/
public function run(AppInterface $application)
public function run(AppInterface $application, $alwaysHandleExceptions = true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since some applications require special exception handling, why not use interface polymorphism that is in place already instead of introducing optional argument and IF here? We already have $application->catchException.

{
$prelaunchSuccessful = $this->preLaunch($application);
if (!$prelaunchSuccessful) {
return;
}

// If ignoring exceptions, don't wrap in a try/catch block so that PHP/Xdebug can output a clean backtrace
if ($this->isDeveloperMode()
&& $this->ignoreExceptionsInDeveloperMode()
// Certain entry points such as get.php and static.php should always be handled by the try/catch block
// below since any exceptions should return a 404 header
&& !$alwaysHandleExceptions
) {
Profiler::start('magento');
$response = $application->launch();
$response->sendResponse();
Profiler::stop('magento');
} else {
try {
try {
Profiler::start('magento');
$response = $application->launch();
$response->sendResponse();
Profiler::stop('magento');
} catch (\Exception $e) {
Profiler::stop('magento');
if (!$application->catchException($this, $e)) {
throw $e;
}
}
} catch (\Exception $e) {
$this->terminate($e);
}
}
}

/**
* Initialize prelaunch sequence to prepare for launching application
*
* All code is being run within a try/catch block (even if $this->ignoreExceptionsInDeveloperMode() is true), as
* this prelaunch code may throw exceptions that must be handled by \Magento\Framework\AppInterface::catchException
*
* @param AppInterface $application
* @return boolean
*/
protected function preLaunch(AppInterface $application)
{
try {
try {
\Magento\Framework\Profiler::start('magento');
\Magento\Framework\Profiler::start('magento_prelaunch');
/**
* It is necessary to initialize error handler even when in developer mode as certain methods require
* errors to be converted into catchable exceptions.
* For example @see \Magento\Framework\View\Model\Layout\Update\Validator::isValid
*/
$this->initErrorHandler();
$this->initObjectManager();
$this->assertMaintenance();
$this->assertInstalled();
$response = $application->launch();
$response->sendResponse();
\Magento\Framework\Profiler::stop('magento');
\Magento\Framework\Profiler::stop('magento_prelaunch');
} catch (\Exception $e) {
\Magento\Framework\Profiler::stop('magento');
\Magento\Framework\Profiler::stop('magento_prelaunch');
if (!$application->catchException($this, $e)) {
throw $e;
}
return false;
}
} catch (\Exception $e) {
$this->terminate($e);
return false;
}
return true;
}

/**
Expand Down Expand Up @@ -440,4 +493,17 @@ protected function terminate(\Exception $e)
}
exit(1);
}

/**
* Whether to handle exceptions in developer mode or to allow PHP to handle display of exceptions
*
* Xdebug provides very detailed stack traces that can assist a developer in troubleshooting an exception. If a
* developer has Xdebug installed and enabled, we should assume that they want to let Xdebug handle exceptions.
*
* @return bool
*/
public function ignoreExceptionsInDeveloperMode()
{
return (function_exists('xdebug_is_enabled') && xdebug_is_enabled());
}
}
47 changes: 47 additions & 0 deletions lib/internal/Magento/Framework/App/Test/Unit/BootstrapTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,53 @@ public function testIsDeveloperModeСontradictoryValues()
$this->assertTrue($bootstrap->isDeveloperMode());
}

public function testIgnoreExceptionsInDeveloperMode()
{
$bootstrap = self::createBootstrap();

$isXdebugInitiallyEnabled = $this->isXdebugEnabled();

$this->ensureXdebugIsDisabled();
$this->assertFalse($bootstrap->ignoreExceptionsInDeveloperMode());

$this->ensureXdebugIsEnabled();
$this->assertTrue($bootstrap->ignoreExceptionsInDeveloperMode());

// Restore Xdebug to initial state
if ($isXdebugInitiallyEnabled) {
$this->ensureXdebugIsEnabled();
} else {
$this->ensureXdebugIsDisabled();
}
}

protected function isXdebugEnabled()
{
if (function_exists('xdebug_is_enabled')) {
return xdebug_is_enabled();
}
return false;
}

protected function ensureXdebugIsDisabled()
{
if (function_exists('xdebug_disable')) {
xdebug_disable();
}
}

protected function ensureXdebugIsEnabled()
{
// While xdebug should not be installed when running tests in a CI environment (like Travis or Atlassian
// Bamboo), it may be enabled on a developer's machine, so this test must account for both states
if (function_exists('xdebug_enable')) {
xdebug_enable();
} else {
// This file creates an "xdebug_is_enabled" function that returns true
require_once __DIR__ . '/_files/other/create_function_xdebug_is_enabled.php';
}
}

public function testRunNoErrors()
{
$responseMock = $this->getMockForAbstractClass('\Magento\Framework\App\ResponseInterface');
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php
/**
* Copyright © 2015 Magento. All rights reserved.
* See COPYING.txt for license details.
*/

if (!function_exists('xdebug_is_enabled')) {
function xdebug_is_enabled() {
return true;
}
}
2 changes: 1 addition & 1 deletion pub/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,4 @@
$bootstrap = \Magento\Framework\App\Bootstrap::create(BP, $params);
/** @var \Magento\Framework\App\Http $app */
$app = $bootstrap->createApplication('Magento\Framework\App\Http');
$bootstrap->run($app);
$bootstrap->run($app, false);