@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator

When a page throws an exception and response construction throws another exception, throw an aggregate exception

Summary:
Depends on D20719. Currently, if a page throws an exception (like a policy exception) and rendering that exception into a response (like a policy dialog) throws another exception (for example, while constructing breadcrumbs), we only show the orginal exception.

This is usually the more useful exception, but sometimes we actually care about the other exception.

Instead of guessing which one is more likely to be useful, throw them both as an "AggregateException" and let the high-level handler flatten it for display.

Test Plan: {F6749312}

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

+52 -8
+10 -4
src/aphront/configuration/AphrontApplicationConfiguration.php
··· 312 312 if ($response_exception) { 313 313 // If we encountered an exception while building a normal response, then 314 314 // encountered another exception while building a response for the first 315 - // exception, just throw the original exception. It is more likely to be 316 - // useful and point at a root cause than the second exception we ran into 317 - // while telling the user about it. 315 + // exception, throw an aggregate exception that will be unpacked by the 316 + // higher-level handler. This is above our pay grade. 318 317 if ($original_exception) { 319 - throw $original_exception; 318 + throw new PhutilAggregateException( 319 + pht( 320 + 'Encountered a processing exception, then another exception when '. 321 + 'trying to build a response for the first exception.'), 322 + array( 323 + $response_exception, 324 + $original_exception, 325 + )); 320 326 } 321 327 322 328 // If we built a response successfully and then ran into an exception
+42 -4
src/aphront/response/AphrontUnhandledExceptionResponse.php
··· 61 61 return 'unhandled-exception'; 62 62 } 63 63 64 + private function getExceptionList() { 65 + return $this->expandException($this->exception); 66 + } 67 + 68 + private function expandException($root) { 69 + if ($root instanceof PhutilAggregateException) { 70 + $list = array(); 71 + 72 + $list[] = $root; 73 + 74 + foreach ($root->getExceptions() as $ex) { 75 + foreach ($this->expandException($ex) as $child) { 76 + $list[] = $child; 77 + } 78 + } 79 + 80 + return $list; 81 + } 82 + 83 + return array($root); 84 + } 85 + 64 86 protected function getResponseBody() { 65 - $ex = $this->exception; 87 + $body = array(); 66 88 89 + foreach ($this->getExceptionList() as $ex) { 90 + $body[] = $this->newHTMLMessage($ex); 91 + } 92 + 93 + return $body; 94 + } 95 + 96 + private function newHTMLMessage($ex) { 67 97 if ($ex instanceof AphrontMalformedRequestException) { 68 98 $title = $ex->getTitle(); 69 99 } else { ··· 122 152 } 123 153 124 154 protected function buildPlainTextResponseString() { 125 - $ex = $this->exception; 155 + $messages = array(); 156 + 157 + foreach ($this->getExceptionList() as $exception) { 158 + $messages[] = $this->newPlainTextMessage($exception); 159 + } 126 160 161 + return implode("\n\n", $messages); 162 + } 163 + 164 + private function newPlainTextMessage($exception) { 127 165 return pht( 128 166 '%s: %s', 129 - get_class($ex), 130 - $ex->getMessage()); 167 + get_class($exception), 168 + $exception->getMessage()); 131 169 } 132 170 133 171 }