Page MenuHomePhorge

Unhandled Exception: Add last call to error message
Needs ReviewPublic

Authored by aklapper on Tue, Apr 8, 18:24.

Details

Summary

Phorge does not expose the location of the last call in unhandled exceptions on the web page. One has to open the error log or console to find the location.
The error message in the yellow box comes directly from PHP's $throwable->getMessage(), and the stacktrace comes directly from PHP's $throwable->getTrace().
Thus manually concatenate the error message string with $throwable->getFile() and $throwable->getLine() to display the location of the last call but strip the absolute path not to expose server details.

Closes T15689

Test Plan

Introduce a random PHP error and load the Phorge page.
For example, add $chart_view += $chart_panel; in src/applications/project/controller/PhabricatorProjectReportsController.php and go to http://phorge.localhost/project/reports/1/

Diff Detail

Repository
rP Phorge
Branch
T15689 (branched from master)
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 1852
Build 1852: arc lint + arc unit

Event Timeline

aklapper requested review of this revision.Tue, Apr 8, 18:24
mainframe98 subscribed.
mainframe98 added inline comments.
src/aphront/handler/PhabricatorDefaultRequestExceptionHandler.php
48

This doesn't work for me. I see Undefined variable $chart_panel at [st/src/error/PhutilErrorHandler.php:273]. That is not that helpful. It's probably caused by the fact that PhutilErrorHandler.php is part of Arcanist.

To reproduce, put $chart_view += $chart_panel; on line 18 of PhabricatorProjectReportsController.php.

51

Given that this information isn't useful to non-developers, it should be behind a check for PhabricatorEnv::getEnvConfig('phabricator.developer-mode') (like below, for the stack trace). Not all installs will want to expose this.

This revision now requires changes to proceed.Wed, Apr 9, 17:42

Thanks! Indeed I should have tested this more.

Need to keep /phorge level in root directory path as it could also be /arcanist.

(I hope that all loaded custom extension paths must be within this directory and not in strange places outside.)

Only expose last call location if phabricator.developer-mode is set

Much better, two nitpicks:

src/aphront/handler/PhabricatorDefaultRequestExceptionHandler.php
48

This works better now, but each path is prepended with a slash; Test at [/phorge/src/applications/project/controller/PhabricatorProjectReportsController.php:18]. This is misleading, because the path is relative. It should omit that: Test at [phorge/src/applications/project/controller/PhabricatorProjectReportsController.php:18].

52

I find the message is easier to read when it is wrapped in quotes: "Test" at [/phorge/src/applications/project/controller/PhabricatorProjectReportsController.php:18].

Strip proceeding slash of path; wrap error message in quotation marks