Improve unhandled exception dialogs

Summary:
Make the unhandled exception dialogs slightly more useful:

  - Make them easier to read.
  - Link to files from Phabricator libraries.
  - Don't show traces by default.
  - Show traces in development mode.
  - Rename button from "Cancel" to "Close" and only show it for Ajax.

Test Plan: Rigged DirectoryHomeController to throw, loaded home page. Changed
stack trace setting in config. Clicked some files in the trace.
Reviewed By: aran
Reviewers: aran, jungejason, tuomaspelkonen, codeblock
CC: aran, epriestley
Differential Revision: 823

conf/default.conf.php

@@ -360,6 +360,10 @@ 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,
+
 
 // -- Files ----------------------------------------------------------------- //
 

conf/development.conf.php

@@ -18,7 +18,8 @@
 
 return array(
 
-  'darkconsole.enabled' => true,
-  'celerity.force-disk-reads' => true,
+  'darkconsole.enabled'           => true,
+  'celerity.force-disk-reads'     => true,
+  'phabricator.show-stack-traces' => true,
 
 ) + phabricator_read_config_file('default');

src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php

@@ -389,14 +389,16 @@ class AphrontDefaultApplicationConfiguration
     $class    = phutil_escape_html(get_class($ex));
     $message  = phutil_escape_html($ex->getMessage());
 
-    $string = (string)$ex;
-    $string = phutil_escape_html($string);
-    $string = str_replace("\n", '<br />', $string);
+    if (PhabricatorEnv::getEnvConfig('phabricator.show-stack-traces')) {
+      $trace = $this->renderStackTrace($ex->getTrace());
+    } else {
+      $trace = null;
+    }
 
     $content =
       '<div class="aphront-unhandled-exception">'.
-        '<h1>Unhandled Exception "'.$class.'": '.$message.'</h1>'.
-        '<code>'.$string.'</code>'.
+        '<div class="exception-message">'.$message.'</div>'.
+        $trace.
       '</div>';
 
     $user = $this->getRequest()->getUser();
@@ -407,11 +409,14 @@ class AphrontDefaultApplicationConfiguration
 
     $dialog = new AphrontDialogView();
     $dialog
-      ->setTitle('Exception!')
+      ->setTitle('Unhandled Exception ("'.$class.'")')
       ->setClass('aphront-exception-dialog')
       ->setUser($user)
-      ->appendChild($content)
-      ->addCancelButton('/');
+      ->appendChild($content);
+
+    if ($this->getRequest()->isAjax()) {
+      $dialog->addCancelButton('/', 'Close');
+    }
 
     $response = new AphrontDialogResponse();
     $response->setDialog($dialog);
@@ -480,4 +485,93 @@ class AphrontDefaultApplicationConfiguration
       ));
   }
 
+  private function renderStackTrace($trace) {
+
+    $libraries = PhutilBootloader::getInstance()->getAllLibraries();
+
+    // TODO: Make this configurable?
+    $host = 'https://secure.phabricator.com';
+
+    $browse = array(
+      'arcanist' =>
+        $host.'/diffusion/ARC/browse/origin:master/src/',
+      'phutil' =>
+        $host.'/diffusion/PHU/browse/origin:master/src/',
+      'phabricator' =>
+        $host.'/diffusion/P/browse/origin:master/src/',
+    );
+
+    $rows = array();
+    $depth = count($trace);
+    foreach ($trace as $part) {
+      $lib = null;
+      $file = $part['file'];
+      $relative = $file;
+      foreach ($libraries as $library) {
+        $root = phutil_get_library_root($library);
+        if (Filesystem::isDescendant($file, $root)) {
+          $lib = $library;
+          $relative = Filesystem::readablePath($file, $root);
+          break;
+        }
+      }
+
+      $where = '';
+      if (isset($part['class'])) {
+        $where .= $part['class'].'::';
+      }
+      if (isset($part['function'])) {
+        $where .= $part['function'].'()';
+      }
+
+      if (isset($browse[$lib])) {
+        $file_name = phutil_render_tag(
+          'a',
+          array(
+            'href' => $browse[$lib].$relative.'$'.$part['line'],
+            'title' => $file,
+            'target' => '_blank',
+          ),
+          phutil_escape_html($relative));
+      } else {
+        $file_name = phutil_render_tag(
+          'span',
+          array(
+            'title' => $file,
+          ),
+          phutil_escape_html($relative));
+      }
+      $file_name = $file_name.' : '.(int)$part['line'];
+
+
+      $rows[] = array(
+        $depth--,
+        phutil_escape_html($lib),
+        $file_name,
+        phutil_escape_html($where),
+      );
+    }
+    $table = new AphrontTableView($rows);
+    $table->setHeaders(
+      array(
+        'Depth',
+        'Library',
+        'File',
+        'Where',
+      ));
+    $table->setColumnClasses(
+      array(
+        'n',
+        '',
+        '',
+        'wide',
+      ));
+
+    return
+      '<div class="exception-trace">'.
+        '<div class="exception-trace-header">Stack Trace</div>'.
+        $table->render().
+      '</div>';
+  }
+
 }

src/aphront/default/configuration/__init__.php

@@ -14,12 +14,16 @@ phutil_require_module('phabricator', 'aphront/response/webpage');
 phutil_require_module('phabricator', 'applications/base/controller/404');
 phutil_require_module('phabricator', 'applications/base/controller/redirect');
 phutil_require_module('phabricator', 'applications/people/storage/user');
+phutil_require_module('phabricator', 'infrastructure/env');
+phutil_require_module('phabricator', 'view/control/table');
 phutil_require_module('phabricator', 'view/dialog');
 phutil_require_module('phabricator', 'view/page/failure');
 phutil_require_module('phabricator', 'view/page/standard');
 
 phutil_require_module('phutil', 'error');
+phutil_require_module('phutil', 'filesystem');
 phutil_require_module('phutil', 'markup');
+phutil_require_module('phutil', 'moduleutils');
 phutil_require_module('phutil', 'utils');
 
 

webroot/rsrc/css/aphront/dialog-view.css

@@ -69,3 +69,22 @@
 .aphront-exception-dialog .aphront-dialog-head {
   background: #aa0000;
 }
+
+.aphront-exception-dialog .exception-message {
+  font-size: 14px;
+  background: #efefef;
+  padding: 1em;
+}
+
+.aphront-exception-dialog .exception-trace {
+  margin-top: 1em;
+  padding: .5em 1em;
+}
+
+.aphront-exception-dialog .exception-trace-header {
+  font-size: 11px;
+  color: #666666;
+  border-bottom: 1px solid #aaaaaa;
+  padding-bottom: .5em;
+  margin-bottom: .5em;
+}