1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-25 16:22:43 +01:00

Let the top-level exception handler dump a stack trace if we reach debug mode before things go sideways

Summary:
Depends on D20140. Ref T13250. Currently, the top-level exception handler doesn't dump stacks because we might not be in debug mode, and we might double-extra-super fatal if we call `PhabricatorEnv:...` to try to figure out if we're in debug mode or not.

We can get around this by setting a flag on the Sink once we're able to confirm that we're in debug mode. Then it's okay for the top-level error handler to show traces.

There's still some small possibility that showing a trace could make us double-super-fatal since we have to call a little more code, but AphrontStackTraceView is pretty conservative about what it does and 99% of the time this is a huge improvement.

Test Plan: {F6205122}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13250

Differential Revision: https://secure.phabricator.com/D20142
This commit is contained in:
epriestley 2019-02-11 13:00:53 -08:00
parent 51cca22d07
commit 187356fea5
7 changed files with 91 additions and 10 deletions

View file

@ -46,7 +46,7 @@ return array(
'rsrc/css/application/config/config-options.css' => '16c920ae', 'rsrc/css/application/config/config-options.css' => '16c920ae',
'rsrc/css/application/config/config-template.css' => '20babf50', 'rsrc/css/application/config/config-template.css' => '20babf50',
'rsrc/css/application/config/setup-issue.css' => '5eed85b2', 'rsrc/css/application/config/setup-issue.css' => '5eed85b2',
'rsrc/css/application/config/unhandled-exception.css' => '9da8fdab', 'rsrc/css/application/config/unhandled-exception.css' => '9ecfc00d',
'rsrc/css/application/conpherence/color.css' => 'b17746b0', 'rsrc/css/application/conpherence/color.css' => 'b17746b0',
'rsrc/css/application/conpherence/durable-column.css' => '2d57072b', 'rsrc/css/application/conpherence/durable-column.css' => '2d57072b',
'rsrc/css/application/conpherence/header-pane.css' => 'c9a3db8e', 'rsrc/css/application/conpherence/header-pane.css' => 'c9a3db8e',
@ -877,7 +877,7 @@ return array(
'syntax-highlighting-css' => '8a16f91b', 'syntax-highlighting-css' => '8a16f91b',
'tokens-css' => 'ce5a50bd', 'tokens-css' => 'ce5a50bd',
'typeahead-browse-css' => 'b7ed02d2', 'typeahead-browse-css' => 'b7ed02d2',
'unhandled-exception-css' => '9da8fdab', 'unhandled-exception-css' => '9ecfc00d',
), ),
'requires' => array( 'requires' => array(
'01384686' => array( '01384686' => array(

View file

@ -118,6 +118,12 @@ final class AphrontApplicationConfiguration
$database_exception = $ex; $database_exception = $ex;
} }
// If we're in developer mode, set a flag so that top-level exception
// handlers can add more information.
if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) {
$sink->setShowStackTraces(true);
}
if ($database_exception) { if ($database_exception) {
$issue = PhabricatorSetupIssue::newDatabaseConnectionIssue( $issue = PhabricatorSetupIssue::newDatabaseConnectionIssue(
$database_exception, $database_exception,

View file

@ -4,8 +4,20 @@ final class AphrontUnhandledExceptionResponse
extends AphrontStandaloneHTMLResponse { extends AphrontStandaloneHTMLResponse {
private $exception; private $exception;
private $showStackTraces;
public function setShowStackTraces($show_stack_traces) {
$this->showStackTraces = $show_stack_traces;
return $this;
}
public function getShowStackTraces() {
return $this->showStackTraces;
}
public function setException($exception) {
// NOTE: We accept an Exception or a Throwable.
public function setException(Exception $exception) {
// Log the exception unless it's specifically a silent malformed request // Log the exception unless it's specifically a silent malformed request
// exception. // exception.
@ -61,10 +73,36 @@ final class AphrontUnhandledExceptionResponse
$body = $ex->getMessage(); $body = $ex->getMessage();
$body = phutil_escape_html_newlines($body); $body = phutil_escape_html_newlines($body);
$classes = array();
$classes[] = 'unhandled-exception-detail';
$stack = null;
if ($this->getShowStackTraces()) {
try {
$stack = id(new AphrontStackTraceView())
->setTrace($ex->getTrace());
$stack = hsprintf('%s', $stack);
$stack = phutil_tag(
'div',
array(
'class' => 'unhandled-exception-stack',
),
$stack);
$classes[] = 'unhandled-exception-with-stack';
} catch (Exception $trace_exception) {
$stack = null;
} catch (Throwable $trace_exception) {
$stack = null;
}
}
return phutil_tag( return phutil_tag(
'div', 'div',
array( array(
'class' => 'unhandled-exception-detail', 'class' => implode(' ', $classes),
), ),
array( array(
phutil_tag( phutil_tag(
@ -79,6 +117,7 @@ final class AphrontUnhandledExceptionResponse
'class' => 'unhandled-exception-body', 'class' => 'unhandled-exception-body',
), ),
$body), $body),
$stack,
)); ));
} }

View file

@ -5,14 +5,22 @@
* Normally this is just @{class:AphrontPHPHTTPSink}, which uses "echo" and * Normally this is just @{class:AphrontPHPHTTPSink}, which uses "echo" and
* "header()" to emit responses. * "header()" to emit responses.
* *
* Mostly, this class allows us to do install security or metrics hooks in the
* output pipeline.
*
* @task write Writing Response Components * @task write Writing Response Components
* @task emit Emitting the Response * @task emit Emitting the Response
*/ */
abstract class AphrontHTTPSink extends Phobject { abstract class AphrontHTTPSink extends Phobject {
private $showStackTraces = false;
final public function setShowStackTraces($show_stack_traces) {
$this->showStackTraces = $show_stack_traces;
return $this;
}
final public function getShowStackTraces() {
return $this->showStackTraces;
}
/* -( Writing Response Components )---------------------------------------- */ /* -( Writing Response Components )---------------------------------------- */

View file

@ -10,7 +10,6 @@ final class AphrontStackTraceView extends AphrontView {
} }
public function render() { public function render() {
$user = $this->getUser();
$trace = $this->trace; $trace = $this->trace;
$libraries = PhutilBootloader::getInstance()->getAllLibraries(); $libraries = PhutilBootloader::getInstance()->getAllLibraries();

View file

@ -44,6 +44,7 @@ try {
try { try {
$response = new AphrontUnhandledExceptionResponse(); $response = new AphrontUnhandledExceptionResponse();
$response->setException($main_exception); $response->setException($main_exception);
$response->setShowStackTraces($sink->getShowStackTraces());
PhabricatorStartup::endOutputCapture(); PhabricatorStartup::endOutputCapture();
$sink->writeResponse($response); $sink->writeResponse($response);

View file

@ -8,12 +8,12 @@
background: #fff; background: #fff;
border: 1px solid #c0392b; border: 1px solid #c0392b;
border-radius: 3px; border-radius: 3px;
padding: 0 8px; padding: 8px;
} }
.unhandled-exception-detail .unhandled-exception-title { .unhandled-exception-detail .unhandled-exception-title {
color: #c0392b; color: #c0392b;
padding: 12px 8px; padding: 4px 8px 12px;
border-bottom: 1px solid #f4dddb; border-bottom: 1px solid #f4dddb;
font-size: 16px; font-size: 16px;
font-weight: 500; font-weight: 500;
@ -23,3 +23,31 @@
.unhandled-exception-detail .unhandled-exception-body { .unhandled-exception-detail .unhandled-exception-body {
padding: 16px 12px; padding: 16px 12px;
} }
.unhandled-exception-with-stack {
max-width: 95%;
}
.unhandled-exception-stack {
background: #fcfcfc;
overflow-x: auto;
overflow-y: hidden;
}
.unhandled-exception-stack table {
border-spacing: 0;
border-collapse: collapse;
width: 100%;
border: 1px solid #d7d7d7;
}
.unhandled-exception-stack th {
background: #e7e7e7;
border-bottom: 1px solid #d7d7d7;
padding: 8px;
}
.unhandled-exception-stack td {
padding: 4px 8px;
white-space: nowrap;
}