From 259638e900deb1e09aa38f0bb4bf5eabf9c20bc5 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 6 Jun 2012 08:15:42 -0700 Subject: [PATCH] Fix minor issues with D2630 Summary: - The config is called "resource-path" and the script references "resource-path", but the actual value checked for is "resource-map". - Use nonempty(), since defaulting with getEnvConfig() will give you null if the setting exists but is set to null. This default is nearly useless so maybe we should change it to use coalesce(). - Remove Celerity map initialization from warmup. We don't currently initialize the environment in warmup, and Celerity initialization now depends on the environment. Test Plan: Ran patch locally and on FPM-Warmup. Reviewers: vrana, btrahan Reviewed By: vrana CC: hsb, aran Differential Revision: https://secure.phabricator.com/D2662 --- conf/default.conf.php | 5 ++--- scripts/fpm/warmup.php | 3 --- src/infrastructure/celerity/CelerityResourceMap.php | 9 +++++---- webroot/index.php | 13 +++++++++++++ 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/conf/default.conf.php b/conf/default.conf.php index d9270c7a24..fe70ae2d32 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -974,9 +974,8 @@ return array( 'phd.start-taskmasters' => 4, // Path to custom celerity resource map. Absolute or relative to - // 'phabricator/src'. Defaults to '__celerity_resource_map__.php'. - // See also `scripts/celerity_mapper.php`. - 'celerity.resource-path' => null, + // 'phabricator/src'. See also `scripts/celerity_mapper.php`. + 'celerity.resource-path' => '__celerity_resource_map__.php', // This value is an input to the hash function when building resource hashes. // It has no security value, but if you accidentally poison user caches (by diff --git a/scripts/fpm/warmup.php b/scripts/fpm/warmup.php index ad0d80c6a4..f4744d1991 100644 --- a/scripts/fpm/warmup.php +++ b/scripts/fpm/warmup.php @@ -48,9 +48,6 @@ function __warmup__() { $loader = new PhutilSymbolLoader(); $loader->selectAndLoadSymbols(); - // Force load of the Celerity map. - CelerityResourceMap::getInstance(); - define('__WARMUP__', true); } diff --git a/src/infrastructure/celerity/CelerityResourceMap.php b/src/infrastructure/celerity/CelerityResourceMap.php index 0221959cc1..9b53660c92 100644 --- a/src/infrastructure/celerity/CelerityResourceMap.php +++ b/src/infrastructure/celerity/CelerityResourceMap.php @@ -35,12 +35,13 @@ final class CelerityResourceMap { if (empty(self::$instance)) { self::$instance = new CelerityResourceMap(); $root = phutil_get_library_root('phabricator'); - $path = PhabricatorEnv::getEnvConfig( - 'celerity.resource-map', - '__celerity_resource_map__.php'); + + $path = PhabricatorEnv::getEnvConfig('celerity.resource-path'); $ok = include_once Filesystem::resolvePath($path, $root); if (!$ok) { - throw new Exception("Failed to load Celerity resource map!"); + throw new Exception( + "Failed to load Celerity resource map! Check the ". + "'celerity.resource-path' setting in your configuration."); } } return self::$instance; diff --git a/webroot/index.php b/webroot/index.php index a2e817c74e..3de50e7738 100644 --- a/webroot/index.php +++ b/webroot/index.php @@ -152,6 +152,10 @@ if ($access_log) { )); } +// If execution throws an exception and then trying to render that exception +// throws another exception, we want to show the original exception, as it is +// likely the root cause of the rendering exception. +$original_exception = null; try { $response = $controller->willBeginExecution(); @@ -172,6 +176,7 @@ try { $response = id(new AphrontRedirectResponse()) ->setURI($ex->getURI()); } catch (Exception $ex) { + $original_exception = $ex; $response = $application->handleException($ex); } @@ -184,6 +189,14 @@ try { if ($access_log) { $access_log->write(); } + if ($original_exception) { + $ex = new PhutilAggregateException( + "Multiple exceptions during processing and rendering.", + array( + $original_exception, + $ex, + )); + } phabricator_fatal('[Rendering Exception] '.$ex->getMessage()); }