Skip to content

Commit 259638e

Browse files
author
epriestley
committed
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
1 parent 3e87112 commit 259638e

File tree

4 files changed

+20
-10
lines changed

4 files changed

+20
-10
lines changed

conf/default.conf.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -974,9 +974,8 @@
974974
'phd.start-taskmasters' => 4,
975975

976976
// Path to custom celerity resource map. Absolute or relative to
977-
// 'phabricator/src'. Defaults to '__celerity_resource_map__.php'.
978-
// See also `scripts/celerity_mapper.php`.
979-
'celerity.resource-path' => null,
977+
// 'phabricator/src'. See also `scripts/celerity_mapper.php`.
978+
'celerity.resource-path' => '__celerity_resource_map__.php',
980979

981980
// This value is an input to the hash function when building resource hashes.
982981
// It has no security value, but if you accidentally poison user caches (by

scripts/fpm/warmup.php

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,6 @@ function __warmup__() {
4848
$loader = new PhutilSymbolLoader();
4949
$loader->selectAndLoadSymbols();
5050

51-
// Force load of the Celerity map.
52-
CelerityResourceMap::getInstance();
53-
5451
define('__WARMUP__', true);
5552
}
5653

src/infrastructure/celerity/CelerityResourceMap.php

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,13 @@ public static function getInstance() {
3535
if (empty(self::$instance)) {
3636
self::$instance = new CelerityResourceMap();
3737
$root = phutil_get_library_root('phabricator');
38-
$path = PhabricatorEnv::getEnvConfig(
39-
'celerity.resource-map',
40-
'__celerity_resource_map__.php');
38+
39+
$path = PhabricatorEnv::getEnvConfig('celerity.resource-path');
4140
$ok = include_once Filesystem::resolvePath($path, $root);
4241
if (!$ok) {
43-
throw new Exception("Failed to load Celerity resource map!");
42+
throw new Exception(
43+
"Failed to load Celerity resource map! Check the ".
44+
"'celerity.resource-path' setting in your configuration.");
4445
}
4546
}
4647
return self::$instance;

webroot/index.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,10 @@
152152
));
153153
}
154154

155+
// If execution throws an exception and then trying to render that exception
156+
// throws another exception, we want to show the original exception, as it is
157+
// likely the root cause of the rendering exception.
158+
$original_exception = null;
155159
try {
156160
$response = $controller->willBeginExecution();
157161

@@ -172,6 +176,7 @@
172176
$response = id(new AphrontRedirectResponse())
173177
->setURI($ex->getURI());
174178
} catch (Exception $ex) {
179+
$original_exception = $ex;
175180
$response = $application->handleException($ex);
176181
}
177182

@@ -184,6 +189,14 @@
184189
if ($access_log) {
185190
$access_log->write();
186191
}
192+
if ($original_exception) {
193+
$ex = new PhutilAggregateException(
194+
"Multiple exceptions during processing and rendering.",
195+
array(
196+
$original_exception,
197+
$ex,
198+
));
199+
}
187200
phabricator_fatal('[Rendering Exception] '.$ex->getMessage());
188201
}
189202

0 commit comments

Comments
 (0)