diff --git a/conf/default.conf.php b/conf/default.conf.php index 6cc166c323..a9e4cff5a1 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -798,13 +798,11 @@ return array( // behalf, silencing the warning. 'phabricator.timezone' => null, - // When unhandled exceptions occur, stack traces are hidden by default. - // You can enable traces for development to make it easier to debug problems. - 'phabricator.show-stack-traces' => false, - - // Shows an error callout if a page generated PHP errors, warnings or notices. - // This makes it harder to miss problems while developing Phabricator. - 'phabricator.show-error-callout' => false, + // Show stack traces when unhandled exceptions occur, force reloading of + // static resources (skipping the cache), show an error callout if a page + // generated PHP errors, warnings, or notices, force disk reads when + // reloading. This option should not be enabled in production. + 'phabricator.developer-mode' => false, // When users write comments which have URIs, they'll be automatically linked // if the protocol appears in this set. This whitelist is primarily to prevent @@ -1220,15 +1218,6 @@ return array( // unlikely that you need to modify this. 'celerity.resource-hash' => 'd9455ea150622ee044f7931dabfa52aa', - // In a development environment, it is desirable to force static resources - // (CSS and JS) to be read from disk on every request, so that edits to them - // appear when you reload the page even if you haven't updated the resource - // maps. This setting ensures requests will be verified against the state on - // disk. Generally, you should leave this off in production (caching behavior - // and performance improve with it off) but turn it on in development. (These - // settings are the defaults.) - 'celerity.force-disk-reads' => false, - // Minify static resources by removing whitespace and comments. You should // enable this in production, but disable it in development. 'celerity.minify' => false, diff --git a/conf/development.conf.php b/conf/development.conf.php index c18382bcf3..704011402c 100644 --- a/conf/development.conf.php +++ b/conf/development.conf.php @@ -2,10 +2,7 @@ return array( + 'phabricator.developer-mode' => true, 'darkconsole.enabled' => true, - 'celerity.force-disk-reads' => true, - 'phabricator.show-stack-traces' => true, - 'phabricator.show-error-callout' => true, - 'celerity.minify' => false, ) + phabricator_read_config_file('default'); diff --git a/src/aphront/AphrontRequest.php b/src/aphront/AphrontRequest.php index c7a659c305..68c0c16b16 100644 --- a/src/aphront/AphrontRequest.php +++ b/src/aphront/AphrontRequest.php @@ -228,6 +228,26 @@ final class AphrontRequest { $more_info = "(This was a web request, {$token_info}.)"; } + // Give a more detailed explanation of how to avoid the exception + // in developer mode. + if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) { + $more_info = $more_info . + "To avoid this error, use phabricator_form() to construct forms. " . + "If you are already using phabricator_form(), make sure the form " . + "'action' uses a relative URI (i.e., begins with a '/'). Forms " . + "using absolute URIs do not include CSRF tokens, to prevent " . + "leaking tokens to external sites.\n\n" . + "If this page performs writes which do not require CSRF " . + "protection (usually, filling caches or logging), you can use " . + "AphrontWriteGuard::beginScopedUnguardedWrites() to temporarily " . + "bypass CSRF protection while writing. You should use this only " . + "for writes which can not be protected with normal CSRF " . + "mechanisms.\n\n" . + "Some UI elements (like PhabricatorActionListView) also have " . + "methods which will allow you to render links as forms (like " . + "setRenderAsForm(true))."; + } + // This should only be able to happen if you load a form, pull your // internet for 6 hours, and then reconnect and immediately submit, // but give the user some indication of what happened since the workflow diff --git a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php index e91884ef58..24a5b9fbc2 100644 --- a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php @@ -235,7 +235,7 @@ class AphrontDefaultApplicationConfiguration "schema is up to date."; } - if (PhabricatorEnv::getEnvConfig('phabricator.show-stack-traces')) { + if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) { $trace = $this->renderStackTrace($ex->getTrace(), $user); } else { $trace = null; diff --git a/src/applications/config/option/PhabricatorDeveloperConfigOptions.php b/src/applications/config/option/PhabricatorDeveloperConfigOptions.php index fba573a602..8f729711e0 100644 --- a/src/applications/config/option/PhabricatorDeveloperConfigOptions.php +++ b/src/applications/config/option/PhabricatorDeveloperConfigOptions.php @@ -86,48 +86,18 @@ final class PhabricatorDeveloperConfigOptions "data to look at eventually). In development, it may be useful to ". "set it to 1 in order to debug performance problems.\n\n". "NOTE: You must install XHProf for profiling to work.")), - $this->newOption('phabricator.show-stack-traces', 'bool', false) + $this->newOption('phabricator.developer-mode', 'bool', false) ->setBoolOptions( array( - pht('Show stack traces'), - pht('Hide stack traces'), + pht('Enable developer mode'), + pht('Disable developer mode'), )) - ->setSummary(pht("Show stack traces when unhandled exceptions occur.")) - ->setDescription( - pht( - "When unhandled exceptions occur, stack traces are hidden by ". - "default. You can enable traces for development to make it easier ". - "to debug problems.")), - $this->newOption('phabricator.show-error-callout', 'bool', false) - ->setBoolOptions( - array( - pht('Show error callout'), - pht('Hide error callout'), - )) - ->setSummary(pht("Show error callout.")) - ->setDescription( - pht( - "Shows an error callout if a page generated PHP errors, warnings ". - "or notices. This makes it harder to miss problems while ". - "developing Phabricator. A callout is simply a red error at the ". - "top of the page.")), - $this->newOption('celerity.force-disk-reads', 'bool', false) - ->setBoolOptions( - array( - pht('Force disk reads'), - pht("Don't force disk reads"), - )) - ->setSummary(pht("Force Celerity to read from disk on every request.")) - ->setDescription( - pht( - "In a development environment, it is desirable to force static ". - "resources (CSS and JS) to be read from disk on every request, so ". - "that edits to them appear when you reload the page even if you ". - "haven't updated the resource maps. This setting ensures requests ". - "will be verified against the state on disk. Generally, you ". - "should leave this off in production (caching behavior and ". - "performance improve with it off) but turn it on in development. ". - "(These settings are the defaults.)")), + ->setSummary(pht("Enable verbose error reporting and disk reads.")) + ->setDescription( + pht( + "This option enables verbose error reporting (stack traces, ". + "error callouts) and forces disk reads of static assets on ". + "every reload.")), $this->newOption('celerity.minify', 'bool', false) ->setBoolOptions( array( diff --git a/src/infrastructure/celerity/CelerityPhabricatorResourceController.php b/src/infrastructure/celerity/CelerityPhabricatorResourceController.php index 761bd3339b..dc0edcfd76 100644 --- a/src/infrastructure/celerity/CelerityPhabricatorResourceController.php +++ b/src/infrastructure/celerity/CelerityPhabricatorResourceController.php @@ -35,7 +35,9 @@ final class CelerityPhabricatorResourceController protected function buildResourceTransformer() { $xformer = new CelerityResourceTransformer(); - $xformer->setMinify(PhabricatorEnv::getEnvConfig('celerity.minify')); + $xformer->setMinify( + !PhabricatorEnv::getEnvConfig('phabricator.developer-mode') && + PhabricatorEnv::getEnvConfig('celerity.minify')); $xformer->setCelerityMap(CelerityResourceMap::getInstance()); return $xformer; } diff --git a/src/infrastructure/celerity/CelerityResourceController.php b/src/infrastructure/celerity/CelerityResourceController.php index 275724bb0e..b3cb96f3bb 100644 --- a/src/infrastructure/celerity/CelerityResourceController.php +++ b/src/infrastructure/celerity/CelerityResourceController.php @@ -35,7 +35,7 @@ abstract class CelerityResourceController extends PhabricatorController { } if (isset($_SERVER['HTTP_IF_MODIFIED_SINCE']) && - !PhabricatorEnv::getEnvConfig('celerity.force-disk-reads')) { + !PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) { // Return a "304 Not Modified". We don't care about the value of this // field since we never change what resource is served by a given URI. return $this->makeResponseCacheable(new Aphront304Response()); diff --git a/src/view/page/PhabricatorBarePageView.php b/src/view/page/PhabricatorBarePageView.php index 9755ab4375..40e6635478 100644 --- a/src/view/page/PhabricatorBarePageView.php +++ b/src/view/page/PhabricatorBarePageView.php @@ -83,7 +83,9 @@ class PhabricatorBarePageView extends AphrontPageView { '', $response->renderResourcesOfType('css'), diff --git a/src/view/page/PhabricatorStandardPageView.php b/src/view/page/PhabricatorStandardPageView.php index f9735a79bc..f932046b56 100644 --- a/src/view/page/PhabricatorStandardPageView.php +++ b/src/view/page/PhabricatorStandardPageView.php @@ -234,7 +234,7 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView { } $developer_warning = null; - if (PhabricatorEnv::getEnvConfig('phabricator.show-error-callout') && + if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode') && DarkConsoleErrorLogPluginAPI::getErrors()) { $developer_warning = '