Modify HttpFetchException handle to log exception

Within the flow of HttpFetchException, the actual exception from curl is preserved and logged. Make HttpFetchException a pretty exception for when it is shown to users.

diff --git a/app/Exceptions/HttpFetchException.php b/app/Exceptions/HttpFetchException.php
index 4ad45d92a35a8449f35e8c8f8ee8db44ce17f4df..c445896c01e13f87b1ad9cd7a7826bccbee6a0ff 100644 (file)
--- a/app/Exceptions/HttpFetchException.php
+++ b/app/Exceptions/HttpFetchException.php
@@ -2,8 +2,22 @@
 
 namespace BookStack\Exceptions;
 
-use Exception;
-
-class HttpFetchException extends Exception
+class HttpFetchException extends PrettyException
 {
+    /**
+     * Construct exception within BookStack\Uploads\HttpFetcher.
+     */
+    public function __construct(string $message = '', int $code = 0, ?\Throwable $previous = null)
+    {
+        parent::__construct($message, $code, $previous);
+
+        if ($previous) {
+            $this->setDetails($previous->getMessage());
+        }
+    }
+
+    public function getStatusCode(): int
+    {
+        return 500;
+    }
 }

diff --git a/app/Uploads/HttpFetcher.php b/app/Uploads/HttpFetcher.php
index 4198bb2a326dce7b3e28d6083a6dc6422daf86ee..fcb4147e9ee65e6b369d947cf706aeac2c438d99 100644 (file)
--- a/app/Uploads/HttpFetcher.php
+++ b/app/Uploads/HttpFetcher.php
@@ -29,7 +29,8 @@ class HttpFetcher
         curl_close($ch);
 
         if ($err) {
-            throw new HttpFetchException($err);
+            $errno = curl_errno($ch);
+            throw new HttpFetchException($err, $errno);
         }
 
         return $data;

diff --git a/app/Uploads/UserAvatars.php b/app/Uploads/UserAvatars.php
index 89e88bd19e02d804fa9132754579a2dcc6c21ab0..1cad3465c785ac5d2c871c542e384dcd48f9d237 100644 (file)
--- a/app/Uploads/UserAvatars.php
+++ b/app/Uploads/UserAvatars.php
@@ -33,8 +33,8 @@ class UserAvatars
             $avatar = $this->saveAvatarImage($user);
             $user->avatar()->associate($avatar);
             $user->save();
-        } catch (Exception $e) {
-            Log::error('Failed to save user avatar image');
+        } catch (HttpFetchException $e) {
+            Log::error('Failed to save user avatar image', ['exception' => $e]);
         }
     }
 
@@ -48,8 +48,8 @@ class UserAvatars
             $avatar = $this->createAvatarImageFromData($user, $imageData, $extension);
             $user->avatar()->associate($avatar);
             $user->save();
-        } catch (Exception $e) {
-            Log::error('Failed to save user avatar image');
+        } catch (HttpFetchException $e) {
+            Log::error('Failed to save user avatar image', ['exception' => $e]);
         }
     }
 
@@ -107,14 +107,14 @@ class UserAvatars
     /**
      * Gets an image from url and returns it as a string of image data.
      *
-     * @throws Exception
+     * @throws HttpFetchException
      */
     protected function getAvatarImageData(string $url): string
     {
         try {
             $imageData = $this->http->fetch($url);
         } catch (HttpFetchException $exception) {
-            throw new Exception(trans('errors.cannot_get_image_from_url', ['url' => $url]));
+            throw new HttpFetchException(trans('errors.cannot_get_image_from_url', ['url' => $url]), $exception->getCode(), $exception);
         }
 
         return $imageData;

diff --git a/tests/Uploads/AvatarTest.php b/tests/Uploads/AvatarTest.php
index 2217cd2bafffa28a79e36c86e2a37a557b7e6ed5..363c1fa95420b858d42980f27e05d8e38e10a07a 100644 (file)
--- a/tests/Uploads/AvatarTest.php
+++ b/tests/Uploads/AvatarTest.php
@@ -4,6 +4,7 @@ namespace Tests\Uploads;
 
 use BookStack\Exceptions\HttpFetchException;
 use BookStack\Uploads\HttpFetcher;
+use BookStack\Uploads\UserAvatars;
 use BookStack\Users\Models\User;
 use Tests\TestCase;
 
@@ -110,4 +111,28 @@ class AvatarTest extends TestCase
         $this->createUserRequest($user);
         $this->assertTrue($logger->hasError('Failed to save user avatar image'));
     }
+
+    public function test_exception_message_on_failed_fetch()
+    {
+        // set wrong url
+        config()->set([
+            'services.disable_services' => false,
+            'services.avatar_url'       => 'http_malformed_url/${email}/${hash}/${size}',
+        ]);
+
+        $user = User::factory()->make();
+        $avatar = app()->make(UserAvatars::class);
+        $url = 'http_malformed_url/' . urlencode(strtolower($user->email)) . '/' . md5(strtolower($user->email)) . '/500';
+        $logger = $this->withTestLogger();
+
+        $avatar->fetchAndAssignToUser($user);
+
+        $this->assertTrue($logger->hasError('Failed to save user avatar image'));
+        $exception = $logger->getRecords()[0]['context']['exception'];
+        $this->assertEquals(new HttpFetchException(
+            'Cannot get image from ' . $url,
+            6,
+            (new HttpFetchException('Could not resolve host: http_malformed_url', 6))
+        ), $exception);
+    }
 }