refactor(logging): log uncaught exceptions to stderr

lib/Common/Logging/ExceptionHandler.php

@@ -26,6 +26,9 @@ public static function Handle($exception)
 
 class WebExceptionHandler extends ExceptionHandler
 {
+    private const ANSI_BOLD_RED = '1;31';
+    private const ANSI_DIM = '2';
+
     /**
      * @var callable
      */
@@ -38,10 +41,7 @@ public function __construct($callback)
 
     public function HandleException($exception)
     {
-        error_log('Error: ' . $exception);
-        ob_start();
-        debug_print_backtrace();
-        error_log(ob_get_clean());
+        $this->WriteToStderr($exception);
 
         // Uncaught exceptions indicate a server-side failure.
         // Set 500 only while headers are still mutable.
@@ -61,6 +61,84 @@ public function HandleException($exception)
 
         call_user_func($this->callback, $errorMessageId);
     }
+
+    /**
+     * Written directly to stderr (not error_log()) so the trace keeps real
+     * newlines instead of Apache stamping its prefix on every line. Colors are
+     * only used when stderr is a terminal. Never throws.
+     */
+    private function WriteToStderr($exception): void
+    {
+        $stderr = false;
+        try {
+            $stderr = $this->OpenStderr();
+            $color = $stderr !== false && function_exists('stream_isatty') && stream_isatty($stderr);
+            $text = self::FormatException($exception, $color);
+            $written = $stderr === false ? false : fwrite($stderr, $text);
+            if ($written !== strlen($text)) {
+                // stderr unavailable or short write: plain text, never ANSI, to the PHP error log
+                error_log($color ? self::FormatException($exception) : $text);
+            }
+        } catch (Throwable $ignored) {
+            // logging must never prevent the error page from rendering
+            try {
+                error_log((string) $exception);
+            } catch (Throwable $alsoIgnored) {
+            }
+        } finally {
+            if (is_resource($stderr)) {
+                fclose($stderr);
+            }
+        }
+    }
+
+    /**
+     * @return resource|false
+     */
+    protected function OpenStderr()
+    {
+        return fopen('php://stderr', 'ab');
+    }
+
+    /**
+     * Formats an exception and its previous-exception chain for stderr.
+     * Control characters in messages are escaped so they cannot forge log
+     * lines or inject terminal escape sequences.
+     *
+     * @param Throwable $exception
+     * @param bool $color
+     * @return string
+     */
+    public static function FormatException($exception, $color = false): string
+    {
+        $style = static fn (string $code, string $text): string => $color ? "\033[{$code}m{$text}\033[0m" : $text;
+        // C0 controls except tab, DEL, C1 controls (U+0080-U+009F) and U+2028/U+2029, matched bytewise
+        // so invalid UTF-8 in a message cannot make the match fail.
+        $clean = static fn (string $text): string => preg_replace_callback(
+            '/[\x00-\x08\x0A-\x1F\x7F]|\xC2[\x80-\x9F]|\xE2\x80[\xA8\xA9]/',
+            static fn (array $m): string => $m[0] === "\n"
+                ? "\n    "
+                : implode('', array_map(static fn (string $b): string => sprintf('\\x%02X', ord($b)), str_split($m[0]))),
+            $text
+        ) ?? '[unprintable]';
+
+        $out = '';
+        for ($e = $exception, $label = 'Uncaught exception'; $e !== null; $e = $e->getPrevious(), $label = 'Caused by') {
+            $out .= $style(self::ANSI_BOLD_RED, sprintf(
+                '%s: %s: %s in %s:%d',
+                $label,
+                $clean(get_class($e)),
+                $clean($e->getMessage()),
+                $clean($e->getFile()),
+                $e->getLine()
+            )) . "\n";
+            foreach (explode("\n", $e->getTraceAsString()) as $line) {
+                $out .= $style(self::ANSI_DIM, ' => ' . $clean($line)) . "\n";
+            }
+        }
+
+        return $out;
+    }
 }
 
 set_exception_handler(['ExceptionHandler', 'Handle']);

tests/lib/Common/Logging/ExceptionHandlerTest.php

@@ -0,0 +1,120 @@
+<?php
+
+declare(strict_types=1);
+
+require_once(ROOT_DIR . 'lib/Common/namespace.php');
+
+class ExceptionHandlerTest extends TestBase
+{
+    public function testIncludesPreviousExceptionChain(): void
+    {
+        $e = new RuntimeException('outer', 0, new LogicException('root cause'));
+
+        $text = WebExceptionHandler::FormatException($e);
+
+        $this->assertStringContainsString('Uncaught exception: RuntimeException: outer', $text);
+        $this->assertStringContainsString('Caused by: LogicException: root cause', $text);
+    }
+
+    public function testEscapesControlCharactersAndNewlines(): void
+    {
+        $e = new Exception("bad\nUncaught exception: forged\033[31m");
+
+        $text = WebExceptionHandler::FormatException($e);
+
+        $this->assertStringNotContainsString("\033", $text);
+        $this->assertStringNotContainsString("\nUncaught exception: forged", $text);
+        $this->assertStringContainsString('\x1B[31m', $text);
+    }
+
+    public function testColorOnlyWhenRequested(): void
+    {
+        $e = new Exception('x');
+
+        $this->assertStringNotContainsString("\033", WebExceptionHandler::FormatException($e));
+        $this->assertStringContainsString("\033[1;31m", WebExceptionHandler::FormatException($e, true));
+    }
+
+    public function testEscapesC1ControlsAndUnicodeLineSeparators(): void
+    {
+        $e = new Exception("a\u{0085}b\u{2028}c\u{009B}d");
+
+        $text = WebExceptionHandler::FormatException($e);
+
+        $this->assertStringNotContainsString("\u{0085}", $text);
+        $this->assertStringNotContainsString("\u{2028}", $text);
+        $this->assertStringNotContainsString("\u{009B}", $text);
+        $this->assertStringContainsString('\xC2\x85', $text);
+    }
+
+    public function testKeepsTabsAndEscapesNulInClassNames(): void
+    {
+        $text = WebExceptionHandler::FormatException(new Exception("a\tb"));
+        $this->assertStringContainsString("a\tb", $text);
+
+        $anonymous = new class ('x') extends Exception {
+        };
+        $this->assertStringNotContainsString("\0", WebExceptionHandler::FormatException($anonymous));
+    }
+
+    public function testFormatsNonExceptionThrowables(): void
+    {
+        $text = WebExceptionHandler::FormatException(new TypeError('boom'));
+
+        $this->assertStringContainsString('Uncaught exception: TypeError: boom', $text);
+    }
+
+    public function testHandleExceptionWritesToStderrStream(): void
+    {
+        $file = tempnam(sys_get_temp_dir(), 'lbexc');
+        $handler = new StreamedWebExceptionHandler(static function () {
+        }, $file);
+
+        try {
+            $handler->HandleException(new Exception('to stream'));
+            $written = file_get_contents($file);
+        } finally {
+            unlink($file);
+        }
+
+        $this->assertStringContainsString('Uncaught exception: Exception: to stream', $written);
+    }
+
+    public function testHandleExceptionFallsBackToErrorLogWhenStderrUnavailable(): void
+    {
+        $logFile = tempnam(sys_get_temp_dir(), 'lbexc');
+        $previous = ini_set('error_log', $logFile);
+        $called = false;
+
+        try {
+            $handler = new StreamedWebExceptionHandler(static function () use (&$called) {
+                $called = true;
+            }, false);
+            $handler->HandleException(new Exception('fallback'));
+            $logged = file_get_contents($logFile);
+        } finally {
+            ini_set('error_log', $previous === false ? '' : $previous);
+            unlink($logFile);
+        }
+
+        $this->assertStringContainsString('Uncaught exception: Exception: fallback', $logged);
+        $this->assertTrue($called, 'error page callback must still run');
+    }
+}
+
+class StreamedWebExceptionHandler extends WebExceptionHandler
+{
+    /** @var string|false path to write to, or false to simulate unavailable stderr */
+    private $target;
+
+    public function __construct($callback, $target)
+    {
+        parent::__construct($callback);
+        $this->target = $target;
+    }
+
+    protected function OpenStderr()
+    {
+        return $this->target === false ? false : fopen($this->target, 'ab');
+    }
+}