Skip to content

Commit cb9db0c

Browse files
simonLeary42simonLeary42
committed
improve logging
not in this version of PHP no args for die replace enable_shutdown_msg with enable_error_to_user only make popup if content already sent prettier conditionals revert die shorten rearrange Apply suggestion from @Copilot Co-authored-by: Copilot <[email protected]> Apply suggestion from @Copilot Co-authored-by: Copilot <[email protected]> Apply suggestion from @Copilot Co-authored-by: Copilot <[email protected]> Apply suggestion from @Copilot Co-authored-by: Copilot <[email protected]> use status code for redirect fix type, revert change
1 parent 7b6b962 commit cb9db0c

File tree

5 files changed

+90
-57
lines changed

5 files changed

+90
-57
lines changed

‎defaults/config.ini.default‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ terms_of_service_url = "https://github.com" ; this can be external or a portal p
1515
account_policy_url = "https://github.com" ; this can be external or a portal page created with "content management"
1616
allow_die = true ; internal use only
1717
enable_verbose_error_log = true ; internal use only
18-
enable_shutdown_msg = true ; internal use only
18+
enable_error_to_user = true ; internal use only
1919

2020
[ldap]
2121
uri = "ldap://identity" ; URI of remote LDAP server

‎deployment/overrides/phpunit/config/config.ini‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,4 @@ custom_user_mappings_dir = "test/custom_user_mappings"
44
[site]
55
allow_die = false
66
enable_verbose_error_log = false
7-
enable_shutdown_message = false
7+
enable_error_to_user = false

‎deployment/overrides/worker/config/config.ini‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
[site]
2-
enable_shutdown_msg = false
2+
enable_error_to_user = false

‎resources/lib/UnitySite.php‎

Lines changed: 86 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -24,85 +24,118 @@ public static function die($x = null, $show_user = false)
2424
}
2525
}
2626

27-
public static function redirect($destination)
27+
public static function redirect($dest)
2828
{
29-
header("Location: $destination");
30-
self::die("Redirect failed, click <a href='$destination'>here</a> to continue.", true);
29+
header("Location: $dest");
30+
self::errorToUser("Redirect failed, click <a href='$dest'>here</a> to continue.", 302);
31+
self::die();
3132
}
3233

33-
private static function headerResponseCode(int $code, string $reason)
34-
{
35-
$protocol = $_SERVER["SERVER_PROTOCOL"] ?? "HTTP/1.1";
36-
$msg = $protocol . " " . strval($code) . " " . $reason;
37-
header($msg, true, $code);
34+
// $data must be JSON serializable
35+
public static function errorLog(
36+
string $title,
37+
string $message,
38+
string|null $errorid = null,
39+
Throwable|null $error = null,
40+
mixed $data = null,
41+
) {
42+
if (!CONFIG["site"]["enable_verbose_error_log"]) {
43+
error_log("$title: $message");
44+
return;
45+
}
46+
$output = [
47+
"message" => $message,
48+
"REMOTE_USER" => $_SERVER["REMOTE_USER"] ?? null,
49+
"REMOTE_ADDR" => $_SERVER["REMOTE_ADDR"] ?? null,
50+
];
51+
if (!is_null($errorid)) {
52+
$output["errorid"] = $errorid;
53+
}
54+
if (!is_null($error)) {
55+
$output["error"] = throwableToArray($error);
56+
} else {
57+
// newlines are bad for error log, but getTrace() is too verbose
58+
$output["trace"] = explode("\n", (new \Exception())->getTraceAsString());
59+
}
60+
if (!is_null($data)) {
61+
$output["data"] = $data;
62+
}
63+
error_log("$title: " . json_encode($output, JSON_UNESCAPED_SLASHES));
3864
}
3965

40-
public static function errorLog(string $title, string $message)
66+
// recursive on $t->getPrevious()
67+
private static function throwableToArray(Throwable $t): array
4168
{
42-
if (CONFIG["site"]["enable_verbose_error_log"] == false) {
43-
error_log("$title: $message");
69+
$output = [
70+
"type" => gettype($t),
71+
"msg" => $t->getMessage(),
72+
// newlines are bad for error log, but getTrace() is too verbose
73+
"trace" => explode("\n", $t->getTraceAsString()),
74+
];
75+
$previous = $t->getPrevious();
76+
if (!is_null($previous)) {
77+
$output["previous"] = throwableToArray($previous);
78+
}
79+
return $output;
80+
}
81+
82+
private static function errorToUser(
83+
string $msg,
84+
int $http_response_code,
85+
string|null $errorid = null
86+
) {
87+
if (!CONFIG["site"]["enable_error_to_user"]) {
4488
return;
4589
}
46-
error_log(
47-
"$title: " . json_encode(
48-
[
49-
"message" => $message,
50-
"REMOTE_USER" => @$_SERVER["REMOTE_USER"],
51-
"REMOTE_ADDR" => @$_SERVER["REMOTE_ADDR"],
52-
"trace" => (new \Exception())->getTraceAsString()
53-
]
54-
)
55-
);
90+
$notes = "Please notify a Unity admin at " . CONFIG["mail"]["support"] . ".";
91+
if (!is_null($errorid)) {
92+
$notes = $notes . " Error ID: $errorid.";
93+
}
94+
if (!headers_sent()) {
95+
http_response_code($http_response_code);
96+
}
97+
// text may not be shown in the webpage in an obvious way, so make a popup
98+
self::alert("$msg $notes");
99+
echo "<h1>$msg</h1><p>$notes</p>";
56100
}
57101

58-
public static function badRequest($message)
102+
public static function badRequest($message, $error = null, $data = null)
59103
{
60-
self::headerResponseCode(400, "bad request");
61-
self::errorLog("bad request", $message);
62-
error_clear_last();
104+
$errorid = uniqid();
105+
self::errorLog("bad request", $message, $errorid, $error, $data);
106+
self::errorToUser("Invalid requested action or submitted data.", 400, $errorid);
63107
self::die($message);
64108
}
65109

66-
public static function forbidden($message)
110+
public static function forbidden($message, $error = null, $data = null)
67111
{
68-
self::headerResponseCode(403, "forbidden");
69-
self::errorLog("forbidden", $message);
70-
error_clear_last();
112+
$errorid = uniqid();
113+
self::errorLog("forbidden", $message, $errorid, $error, $data);
114+
self::errorToUser("Permission denied.", 403, $errorid);
115+
self::die($message);
116+
}
117+
118+
public static function internalServerError($message, $error = null, $data = null)
119+
{
120+
$errorid = uniqid();
121+
self::errorLog("internal server error", $message, $errorid, $error, $data);
122+
self::errorToUser("An internal server error has occurred.", 500, $errorid);
71123
self::die($message);
72124
}
73125

74126
// https://www.php.net/manual/en/function.register-shutdown-function.php
75127
public static function shutdown()
76128
{
77-
if (CONFIG["site"]["enable_shutdown_msg"] == false) {
78-
return;
79-
}
80129
$e = error_get_last();
81130
if (is_null($e) || $e["type"] !== E_ERROR) {
82131
return;
83132
}
84-
if (!headers_sent()) {
85-
self::headerResponseCode(500, "internal server error");
133+
// newlines are bad for error log
134+
if (!is_null($e) && array_key_exists("message", $e) && str_contains($e["message"], "\n")) {
135+
$e["message"] = explode("\n", $e["message"]);
86136
}
87-
$errorid = uniqid();
88-
$e["unity_error_id"] = $errorid;
89-
self::errorLog("internal server error", json_encode($e));
90-
echo "
91-
<h1>An internal server error has occurred.</h1>
92-
<p>
93-
Please notify a Unity admin at "
94-
. CONFIG["mail"]["support"]
95-
. ". Error ID: $errorid.
96-
</p>
97-
";
98-
// if content already printed, status code will be ignored and alert text may not be
99-
// shown in the webpage in an obvious way, so make a popup
100-
self::alert(
101-
"An internal server error has occurred. "
102-
. "Please notify a Unity admin at "
103-
. CONFIG["mail"]["support"]
104-
. ". Error ID: $errorid."
105-
);
137+
// error_get_last is an array, not a Throwable
138+
self::internalServerError("An internal server error has occurred.", data: ["error" => $e]);
106139
}
107140

108141
public static function arrayGetOrBadRequest(array $array, ...$keys)

‎resources/lib/phpopenldaper‎

0 commit comments

Comments
 (0)