1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-22 14:52:41 +01:00

Try harder to present display/rendering exceptions to the user using standard exception handling

Summary:
Ref T13250. When exceptions occur in display/rendering/writing, they currently go straight to the fallback handler. This is a minimal handler which doesn't show a stack trace or include any debugging details.

In some cases, we have to do this: some of these exceptions prevent us from building a normal page. For example, if the menu bar has a hard fatal in it, we aren't going to be able to build a nice exception page with a menu bar no matter how hard we try.

However, in many cases the error is mundane: something detected something invalid and raised an exception during rendering. In these cases there's no problem with the page chrome or the rendering pathway itself, just with rendering the page data.

When we get a rendering/response exception, try a second time to build a nice normal exception page. This will often work. If it doesn't work, fall back as before.

Test Plan:
  - Forced the error from T13250 by applying D20136 but not D20134.
  - Before:

{F6205001}

  - After:

{F6205002}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13250

Differential Revision: https://secure.phabricator.com/D20137
This commit is contained in:
epriestley 2019-02-11 09:32:33 -08:00
parent 1275326ea6
commit e03079aaaa

View file

@ -282,23 +282,62 @@ final class AphrontApplicationConfiguration
} }
} catch (Exception $ex) { } catch (Exception $ex) {
$original_exception = $ex; $original_exception = $ex;
$response = $this->handleThrowable($ex);
} catch (Throwable $ex) { } catch (Throwable $ex) {
$original_exception = $ex; $original_exception = $ex;
$response = $this->handleThrowable($ex);
} }
try { try {
if ($original_exception) {
$response = $this->handleThrowable($original_exception);
}
$response = $this->produceResponse($request, $response); $response = $this->produceResponse($request, $response);
$response = $controller->willSendResponse($response); $response = $controller->willSendResponse($response);
$response->setRequest($request); $response->setRequest($request);
self::writeResponse($sink, $response); self::writeResponse($sink, $response);
} catch (Exception $ex) { } catch (Exception $response_exception) {
// If we encountered an exception while building a normal response, then
// encountered another exception while building a response for the first
// exception, just throw the original exception. It is more likely to be
// useful and point at a root cause than the second exception we ran into
// while telling the user about it.
if ($original_exception) { if ($original_exception) {
throw $original_exception; throw $original_exception;
} }
throw $ex;
// If we built a response successfully and then ran into an exception
// trying to render it, try to handle and present that exception to the
// user using the standard handler.
// The problem here might be in rendering (more common) or in the actual
// response mechanism (less common). If it's in rendering, we can likely
// still render a nice exception page: the majority of rendering issues
// are in main page content, not content shared with the exception page.
$handling_exception = null;
try {
$response = $this->handleThrowable($response_exception);
$response = $this->produceResponse($request, $response);
$response = $controller->willSendResponse($response);
$response->setRequest($request);
self::writeResponse($sink, $response);
} catch (Exception $ex) {
$handling_exception = $ex;
} catch (Throwable $ex) {
$handling_exception = $ex;
}
// If we didn't have any luck with that, raise the original response
// exception. As above, this is the root cause exception and more likely
// to be useful. This will go to the fallback error handler at top
// level.
if ($handling_exception) {
throw $response_exception;
}
} }
return $response; return $response;