From 6f388f97d9592fcd28c6dff301c204c521eec59b Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sat, 16 Jul 2011 07:49:04 -0700
Subject: [PATCH] Improve Phabricator behavior for fatal errors

Summary:
  - Exceptions on the rendering pathway currently go uncaught and result in a
blank page. Commonly, this is a bad require_celerity_resource() call. Although
we can't safely render a page if the rendering pathway is broken, we can show a
useful message.
  - When PHP exits because of a fatal error, there is an opportunity to run code
in the shutdown handler. This allows us to show messages at least some of the
time, e.g. "call to unknown function derp() in somefile.php at line 99"
  - flip dem tables

Test Plan: Added fatals ("derp();") and rendering exceptions
("require_celerity_resource('does-not-exist')") to a controller and verified
that the error handling behavior is now more useful.
Reviewed By: aran
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley
Differential Revision: 680
---
 .../celerity/map/CelerityResourceMap.php      |  6 +-
 webroot/index.php                             | 67 +++++++++++++++----
 2 files changed, 56 insertions(+), 17 deletions(-)

diff --git a/src/infrastructure/celerity/map/CelerityResourceMap.php b/src/infrastructure/celerity/map/CelerityResourceMap.php
index 59fff93f34..5d0ec732fc 100644
--- a/src/infrastructure/celerity/map/CelerityResourceMap.php
+++ b/src/infrastructure/celerity/map/CelerityResourceMap.php
@@ -63,7 +63,7 @@ final class CelerityResourceMap {
   private function resolveResource(array &$map, $symbol) {
     if (empty($this->resourceMap[$symbol])) {
       throw new Exception(
-        "Attempting to resolve unknown resource, '{$symbol}'.");
+        "Attempting to resolve unknown Celerity resource, '{$symbol}'.");
     }
 
     $info = $this->resourceMap[$symbol];
@@ -94,8 +94,8 @@ final class CelerityResourceMap {
         $package = $this->packageMap['reverse'][$symbol];
         $package_info = $this->packageMap['packages'][$package];
         $packaged[$package_info['name']] = $package_info;
-        foreach ($package_info['symbols'] as $symbol) {
-          $handled[$symbol] = true;
+        foreach ($package_info['symbols'] as $packaged_symbol) {
+          $handled[$packaged_symbol] = true;
         }
       }
     }
diff --git a/webroot/index.php b/webroot/index.php
index 99420087f8..c9912c5cfc 100644
--- a/webroot/index.php
+++ b/webroot/index.php
@@ -55,6 +55,8 @@ if (get_magic_quotes_gpc()) {
     "disable it to run Phabricator. Consult the PHP manual for instructions.");
 }
 
+register_shutdown_function('phabricator_shutdown');
+
 require_once dirname(dirname(__FILE__)).'/conf/__init_conf__.php';
 
 try {
@@ -74,8 +76,7 @@ try {
   PhutilErrorHandler::initialize();
 
 } catch (Exception $ex) {
-  phabricator_fatal_config_error(
-    "[Exception] ".$ex->getMessage());
+  phabricator_fatal("[Initialization Exception] ".$ex->getMessage());
 }
 
 $tz = PhabricatorEnv::getEnvConfig('phabricator.timezone');
@@ -130,11 +131,13 @@ try {
   $response = $application->handleException($ex);
 }
 
-$response = $application->willSendResponse($response);
-
-$response->setRequest($request);
-
-$response_string = $response->buildResponseString();
+try {
+  $response = $application->willSendResponse($response);
+  $response->setRequest($request);
+  $response_string = $response->buildResponseString();
+} catch (Exception $ex) {
+  phabricator_fatal('[Rendering Exception] '.$ex->getMessage());
+}
 
 $code = $response->getHTTPResponseCode();
 if ($code != 200) {
@@ -172,7 +175,6 @@ if (isset($_REQUEST['__profile__']) &&
 
 echo $response_string;
 
-
 /**
  * @group aphront
  */
@@ -198,15 +200,11 @@ function setup_aphront_basics() {
   // directories).
   phutil_load_library($aphront_root.'/src');
   phutil_load_library('arcanist/src');
+
 }
 
 function phabricator_fatal_config_error($msg) {
-  header('Content-Type: text/plain', $replace = true, $http_error = 500);
-  $error = "CONFIG ERROR: ".$msg."\n";
-
-  error_log($error);
-  echo $error;
-
+  phabricator_fatal("CONFIG ERROR: ".$msg."\n");
   die();
 }
 
@@ -237,3 +235,44 @@ function phabricator_detect_insane_memory_limit() {
     "(\"{$memory_limit}\"). Set it to a more reasonable value (it must be no ".
     "more than {$char_limit} characters long).");
 }
+
+function phabricator_shutdown() {
+  $event = error_get_last();
+
+  if (!$event) {
+    return;
+  }
+
+  if ($event['type'] != E_ERROR) {
+    return;
+  }
+
+  $msg = ">>> UNRECOVERABLE FATAL ERROR <<<\n\n";
+  if ($event) {
+    // Even though we should be emitting this as text-plain, escape things just
+    // to be sure since we can't really be sure what the program state is when
+    // we get here.
+    $msg .= phutil_escape_html($event['message'])."\n\n";
+    $msg .= phutil_escape_html($event['file'].':'.$event['line']);
+  }
+
+  // flip dem tables
+  $msg .= "\n\n\n";
+  $msg .= "\xe2\x94\xbb\xe2\x94\x81\xe2\x94\xbb\x20\xef\xb8\xb5\x20\xc2\xaf".
+          "\x5c\x5f\x28\xe3\x83\x84\x29\x5f\x2f\xc2\xaf\x20\xef\xb8\xb5\x20".
+          "\xe2\x94\xbb\xe2\x94\x81\xe2\x94\xbb";
+
+  phabricator_fatal($msg);
+}
+
+function phabricator_fatal($msg) {
+  header(
+    'Content-Type: text/plain; charset=utf-8',
+    $replace = true,
+    $http_error = 500);
+
+  error_log($msg);
+  echo $msg;
+
+  exit(1);
+}