From 4c3ebb312d14f9aa5e371fb359398590d2117330 Mon Sep 17 00:00:00 2001
From: Dag <me@dvikan.no>
Date: Mon, 20 Mar 2023 19:11:51 +0100
Subject: [PATCH] feat: improve error handling ux (#3298)

* feat: improve error handling ux

* feat: add error messages for failed xml parsing
---
 lib/FeedExpander.php     |  4 ++--
 lib/contents.php         | 43 +++++++++++++++++++--------------------
 lib/utils.php            |  6 +++++-
 templates/error.html.php | 44 ++++++++++++++++++++++++++++++++++++++--
 4 files changed, 70 insertions(+), 27 deletions(-)

diff --git a/lib/FeedExpander.php b/lib/FeedExpander.php
index caface38..edfe99d4 100644
--- a/lib/FeedExpander.php
+++ b/lib/FeedExpander.php
@@ -102,7 +102,7 @@ abstract class FeedExpander extends BridgeAbstract
         $httpHeaders = ['Accept: ' . implode(', ', $mimeTypes)];
         $content = getContents($url, $httpHeaders);
         if ($content === '') {
-            throw new \Exception(sprintf('Unable to parse xml from `%s` because we got the empty string', $url));
+            throw new \Exception(sprintf('Unable to parse xml from `%s` because we got the empty string', $url), 10);
         }
         // Maybe move this call earlier up the stack frames
         // Disable triggering of the php error-handler and handle errors manually instead
@@ -121,7 +121,7 @@ abstract class FeedExpander extends BridgeAbstract
                 // Render only the first error into exception message
                 $firstXmlErrorMessage = $xmlErrors[0]->message;
             }
-            throw new \Exception(sprintf('Unable to parse xml from `%s` %s', $url, $firstXmlErrorMessage ?? ''));
+            throw new \Exception(sprintf('Unable to parse xml from `%s` %s', $url, $firstXmlErrorMessage ?? ''), 11);
         }
         // Restore previous behaviour in case other code relies on it being off
         libxml_use_internal_errors(false);
diff --git a/lib/contents.php b/lib/contents.php
index 5dec2730..7f5ec2f2 100644
--- a/lib/contents.php
+++ b/lib/contents.php
@@ -178,29 +178,28 @@ function getContents(
             $response['content'] = $cache->loadData();
             break;
         default:
-            if (Debug::isEnabled()) {
-                // Include a part of the response body in the exception message
-                throw new HttpException(
-                    sprintf(
-                        '%s resulted in `%s %s: %s`',
-                        $url,
-                        $result['code'],
-                        Response::STATUS_CODES[$result['code']] ?? '',
-                        mb_substr($result['body'], 0, 500),
-                    ),
-                    $result['code']
-                );
-            } else {
-                throw new HttpException(
-                    sprintf(
-                        '%s resulted in `%s %s`',
-                        $url,
-                        $result['code'],
-                        Response::STATUS_CODES[$result['code']] ?? '',
-                    ),
-                    $result['code']
-                );
+            $exceptionMessage = sprintf(
+                '%s resulted in %s %s %s',
+                $url,
+                $result['code'],
+                Response::STATUS_CODES[$result['code']] ?? '',
+                // If debug, include a part of the response body in the exception message
+                Debug::isEnabled() ? mb_substr($result['body'], 0, 500) : '',
+            );
+
+            // The following code must be extracted if it grows too much
+            $cloudflareTitles = [
+                '<title>Just a moment...',
+                '<title>Please Wait...',
+                '<title>Attention Required!'
+            ];
+            foreach ($cloudflareTitles as $cloudflareTitle) {
+                if (str_contains($result['body'], $cloudflareTitle)) {
+                    throw new CloudFlareException($exceptionMessage, $result['code']);
+                }
             }
+
+            throw new HttpException($exceptionMessage, $result['code']);
     }
     if ($returnFull === true) {
         return $response;
diff --git a/lib/utils.php b/lib/utils.php
index db9db0b5..ea329f8d 100644
--- a/lib/utils.php
+++ b/lib/utils.php
@@ -1,6 +1,10 @@
 <?php
 
-final class HttpException extends \Exception
+class HttpException extends \Exception
+{
+}
+
+final class CloudFlareException extends HttpException
 {
 }
 
diff --git a/templates/error.html.php b/templates/error.html.php
index fb94fc25..5220ba7c 100644
--- a/templates/error.html.php
+++ b/templates/error.html.php
@@ -1,8 +1,48 @@
 <div class="error">
 
-    <h1>Application Error</h1>
+    <?php if ($e instanceof HttpException): ?>
+        <?php if ($e instanceof CloudFlareException): ?>
+            <h2>The website is protected by CloudFlare</h2>
+            <p>
+                RSS-Bridge tried to fetch a website.
+                The fetching was blocked by CloudFlare.
+                CloudFlare is anti-bot software.
+                Its purpose is to block non-humans.
+            </p>
+        <?php endif; ?>
 
-    <p>The application could not run because of the following error:</p>
+        <?php if ($e->getCode() === 404): ?>
+            <h2>The website was 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>
+            <p>
+                RSS-Bridge tried to fetch a website.
+                They told us to try again later.
+            </p>
+        <?php endif; ?>
+
+    <?php else: ?>
+        <?php if ($e->getCode() === 10): ?>
+            <h2>The rss feed is completely empty</h2>
+            <p>
+                RSS-Bridge tried parse the empty string as xml.
+                The fetched url is not pointing to real xml.
+            </p>
+        <?php endif; ?>
+
+        <?php if ($e->getCode() === 11): ?>
+            <h2>There is something wrong with the rss feed</h2>
+            <p>
+                RSS-Bridge tried parse xml. It failed. The xml is probably broken.
+            </p>
+        <?php endif; ?>
+    <?php endif; ?>
 
     <h2>Details</h2>