feat: improve error/exception ui

dvikan · Sep 23, 2023 · 987ed1f · 987ed1f
1 parent cb6c931
commit 987ed1f
Show file tree

Hide file tree

Showing 9 changed files with 54 additions and 24 deletions.
diff --git a/actions/ConnectivityAction.php b/actions/ConnectivityAction.php
@@ -34,7 +34,7 @@ public function __construct()
     public function execute(array $request)
     {
         if (!Debug::isEnabled()) {
-            return new Response('This action is only available in debug mode!');
+            return new Response('This action is only available in debug mode!', 403);
         }
 
         $bridgeName = $request['bridge'] ?? null;

diff --git a/actions/DisplayAction.php b/actions/DisplayAction.php
@@ -14,7 +14,10 @@ public function __construct()
     public function execute(array $request)
     {
         if (Configuration::getConfig('system', 'enable_maintenance_mode')) {
-            return new Response('503 Service Unavailable', 503);
+            return new Response(render(__DIR__ . '/../templates/error.html.php', [
+                'title'     => '503 Service Unavailable',
+                'message'   => 'RSS-Bridge is down for maintenance.',
+            ]), 503);
         }
         $cacheKey = 'http_' . json_encode($request);
         /** @var Response $cachedResponse */
@@ -36,19 +39,19 @@ public function execute(array $request)
 
         $bridgeName = $request['bridge'] ?? null;
         if (!$bridgeName) {
-            return new Response('Missing bridge param', 400);
+            return new Response(render(__DIR__ . '/../templates/error.html.php', ['message' => 'Missing bridge parameter']), 400);
         }
         $bridgeFactory = new BridgeFactory();
         $bridgeClassName = $bridgeFactory->createBridgeClassName($bridgeName);
         if (!$bridgeClassName) {
-            return new Response('Bridge not found', 404);
+            return new Response(render(__DIR__ . '/../templates/error.html.php', ['message' => 'Bridge not found']), 404);
         }
         $format = $request['format'] ?? null;
         if (!$format) {
-            return new Response('You must specify a format!', 400);
+            return new Response(render(__DIR__ . '/../templates/error.html.php', ['message' => 'You must specify a format']), 400);
         }
         if (!$bridgeFactory->isEnabled($bridgeClassName)) {
-            return new Response('This bridge is not whitelisted', 400);
+            return new Response(render(__DIR__ . '/../templates/error.html.php', ['message' => 'This bridge is not whitelisted']), 400);
         }
 
         $noproxy = $request['_noproxy'] ?? null;
@@ -120,11 +123,11 @@ private function createResponse(array $request, BridgeAbstract $bridge, FormatIn
                 // Reproduce (and log) these responses regardless of error output and report limit
                 if ($e->getCode() === 429) {
                     $this->logger->info(sprintf('Exception in DisplayAction(%s): %s', $bridge->getShortName(), create_sane_exception_message($e)));
-                    return new Response('429 Too Many Requests', 429);
+                    return new Response(render(__DIR__ . '/../templates/exception.html.php', ['e' => $e]), 429);
                 }
                 if ($e->getCode() === 503) {
                     $this->logger->info(sprintf('Exception in DisplayAction(%s): %s', $bridge->getShortName(), create_sane_exception_message($e)));
-                    return new Response('503 Service Unavailable', 503);
+                    return new Response(render(__DIR__ . '/../templates/exception.html.php', ['e' => $e]), 503);
                 }
             }
             $this->logger->error(sprintf('Exception in DisplayAction(%s)', $bridge->getShortName()), ['e' => $e]);
@@ -140,7 +143,7 @@ private function createResponse(array $request, BridgeAbstract $bridge, FormatIn
                     // Render the exception as a feed item
                     $items[] = $this->createFeedItemFromException($e, $bridge);
                 } elseif ($errorOutput === 'http') {
-                    return new Response(render(__DIR__ . '/../templates/error.html.php', ['e' => $e]), 500);
+                    return new Response(render(__DIR__ . '/../templates/exception.html.php', ['e' => $e]), 500);
                 } elseif ($errorOutput === 'none') {
                     // Do nothing (produces an empty feed)
                 }
@@ -173,7 +176,7 @@ private function createFeedItemFromException($e, BridgeAbstract $bridge): FeedIt
         $item->setUid($bridge->getName() . '_' . $uniqueIdentifier);
 
         $content = render_template(__DIR__ . '/../templates/bridge-error.html.php', [
-            'error' => render_template(__DIR__ . '/../templates/error.html.php', ['e' => $e]),
+            'error' => render_template(__DIR__ . '/../templates/exception.html.php', ['e' => $e]),
             'searchUrl' => self::createGithubSearchUrl($bridge),
             'issueUrl' => self::createGithubIssueUrl($bridge, $e, create_sane_exception_message($e)),
             'maintainer' => $bridge->getMaintainer(),

diff --git a/lib/AuthenticationMiddleware.php b/lib/AuthenticationMiddleware.php
@@ -44,6 +44,8 @@ private function renderAuthenticationDialog(): string
     {
         http_response_code(401);
         header('WWW-Authenticate: Basic realm="RSS-Bridge"');
-        return render('access-denied.html.php');
+        return render(__DIR__ . '/../templates/error.html.php', [
+            'message' => 'Please authenticate in order to access this instance!',
+        ]);
     }
 }
diff --git a/lib/RssBridge.php b/lib/RssBridge.php
@@ -22,7 +22,7 @@ public function __construct()
         set_exception_handler(function (\Throwable $e) {
             self::$logger->error('Uncaught Exception', ['e' => $e]);
             http_response_code(500);
-            print render(__DIR__ . '/../templates/error.html.php', ['e' => $e]);
+            print render(__DIR__ . '/../templates/exception.html.php', ['e' => $e]);
             exit(1);
         });
 
@@ -117,7 +117,7 @@ public function main(array $argv = []): void
         } catch (\Throwable $e) {
             self::$logger->error('Exception in RssBridge::main()', ['e' => $e]);
             http_response_code(500);
-            print render(__DIR__ . '/../templates/error.html.php', ['e' => $e]);
+            print render(__DIR__ . '/../templates/exception.html.php', ['e' => $e]);
         }
     }
 

diff --git a/lib/html.php b/lib/html.php
@@ -34,8 +34,9 @@ function render(string $template, array $context = []): string
 }
 
 /**
- * Render template as absolute path or relative to templates folder.
- * Do not pass user input in $template
+ * Render php template with context
+ *
+ * DO NOT PASS USER INPUT IN $template or $context
  */
 function render_template(string $template, array $context = []): string
 {

diff --git a/lib/http.php b/lib/http.php
@@ -115,8 +115,8 @@ public function request(string $url, array $config = []): Response
         $attempts = 0;
         while (true) {
             $attempts++;
-            $data = curl_exec($ch);
-            if ($data !== false) {
+            $body = curl_exec($ch);
+            if ($body !== false) {
                 // The network call was successful, so break out of the loop
                 break;
             }
@@ -136,7 +136,7 @@ public function request(string $url, array $config = []): Response
 
         $statusCode = curl_getinfo($ch, CURLINFO_RESPONSE_CODE);
         curl_close($ch);
-        return new Response($data, $statusCode, $responseHeaders);
+        return new Response($body, $statusCode, $responseHeaders);
     }
 }
 

diff --git a/templates/access-denied.html.php b/templates/access-denied.html.php
diff --git a/templates/error.php b/templates/error.php
@@ -0,0 +1,15 @@
+<?php
+/**
+ * This template is for rendering error messages (not exceptions)
+ */
+?>
+
+<?php if (isset($title)): ?>
+    <h1>
+        <?= e($title) ?>
+    </h1>
+<?php endif; ?>
+
+<p>
+    <?= e($message) ?>
+</p>
diff --git a/templates/error.html.php → templates/exception.html.php b/templates/error.html.php → templates/exception.html.php
@@ -1,3 +1,8 @@
+<?php
+/**
+ * This template is used for rendering exceptions
+ */
+?>
 <div class="error">
 
     <?php if ($e instanceof HttpException): ?>
@@ -12,21 +17,29 @@
         <?php endif; ?>
 
         <?php if ($e->getCode() === 404): ?>
-            <h2>The website was not found</h2>
+            <h2>404 Page Not Found</h2>
             <p>
                 RSS-Bridge tried to fetch a page on a website.
                 But it doesn't exists.
             </p>
         <?php endif; ?>
 
         <?php if ($e->getCode() === 429): ?>
-            <h2>Try again later</h2>
+            <h2>429 Try again later</h2>
             <p>
                 RSS-Bridge tried to fetch a website.
                 They told us to try again later.
             </p>
         <?php endif; ?>
 
+        <?php if ($e->getCode() === 503): ?>
+            <h2>503 Service Unavailable</h2>
+            <p>
+                Common causes are a server that is down for maintenance
+                or that is overloaded.
+            </p>
+        <?php endif; ?>
+
     <?php else: ?>
         <?php if ($e->getCode() === 10): ?>
             <h2>The rss feed is completely empty</h2>