Improve exception reporting behavior for core exceptions

Summary:
See <https://github.com/facebook/phabricator/issues/487>. If an exception is thrown too high in the stack for the main exception handling to deal with it, we currently never report a stack trace. Instead:

  - Always report a stack trace to the error log.
  - With developer mode, also report a stack trace to the screen.

Test Plan: Added a high-level `throw` and hit both cases. Got traces in the log and traces-under-developer-mode on screen.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D8022

support/PhabricatorStartup.php

@@ -198,9 +198,54 @@ final class PhabricatorStartup {
 
 
   /**
+   * Fatal the request completely in response to an exception, sending a plain
+   * text message to the client. Calls @{method:didFatal} internally.
+   *
+   * @param   string    Brief description of the exception context, like
+   *                    `"Rendering Exception"`.
+   * @param   Exception The exception itself.
+   * @param   bool      True if it's okay to show the exception's stack trace
+   *                    to the user. The trace will always be logged.
+   * @return  exit      This method **does not return**.
+   *
    * @task apocalypse
    */
-  public static function didFatal($message) {
+  public static function didEncounterFatalException(
+    $note,
+    Exception $ex,
+    $show_trace) {
+
+    $message = '['.$note.'/'.get_class($ex).'] '.$ex->getMessage();
+
+    $full_message = $message;
+    $full_message .= "\n\n";
+    $full_message .= $ex->getTraceAsString();
+
+    if ($show_trace) {
+      $message = $full_message;
+    }
+
+    self::didFatal($message, $full_message);
+  }
+
+
+  /**
+   * Fatal the request completely, sending a plain text message to the client.
+   *
+   * @param   string  Plain text message to send to the client.
+   * @param   string  Plain text message to send to the error log. If not
+   *                  provided, the client message is used. You can pass a more
+   *                  detailed message here (e.g., with stack traces) to avoid
+   *                  showing it to users.
+   * @return  exit    This method **does not return**.
+   *
+   * @task apocalypse
+   */
+  public static function didFatal($message, $log_message = null) {
+    if ($log_message === null) {
+      $log_message = $message;
+    }
+
     self::endOutputCapture();
     $access_log = self::getGlobal('log.access');
 
@@ -219,7 +264,7 @@ final class PhabricatorStartup {
       $replace = true,
       $http_error = 500);
 
-    error_log($message);
+    error_log($log_message);
     echo $message;
 
     exit(1);

webroot/index.php

@@ -3,10 +3,13 @@
 require_once dirname(dirname(__FILE__)).'/support/PhabricatorStartup.php';
 PhabricatorStartup::didStartup();
 
+$show_unexpected_traces = false;
 try {
   PhabricatorStartup::loadCoreLibraries();
 
   PhabricatorEnv::initializeWebEnvironment();
+  $show_unexpected_traces = PhabricatorEnv::getEnvConfig(
+    'phabricator.developer-mode');
 
   // This is the earliest we can get away with this, we need env config first.
   PhabricatorAccessLog::init();
@@ -124,7 +127,10 @@ try {
           $ex,
         ));
     }
-    PhabricatorStartup::didFatal('[Rendering Exception] '.$ex->getMessage());
+    PhabricatorStartup::didEncounterFatalException(
+      'Rendering Exception',
+      $ex,
+      $show_unexpected_traces);
   }
 
   $write_guard->dispose();
@@ -137,6 +143,9 @@ try {
 
   DarkConsoleXHProfPluginAPI::saveProfilerSample($access_log);
 } catch (Exception $ex) {
-  PhabricatorStartup::didFatal("[Exception] ".$ex->getMessage());
+  PhabricatorStartup::didEncounterFatalException(
+    'Core Exception',
+    $ex,
+    $show_unexpected_traces);
 }