Merge pull request #4317 from devdot/http-fetch-improve-exception-logging

Modify HttpFetchException flow to log the exception

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;

app/Uploads/UserAvatars.php

@@ -34,7 +34,7 @@ class UserAvatars
             $user->avatar()->associate($avatar);
             $user->save();
         } catch (Exception $e) {
-            Log::error('Failed to save user avatar image');
+            Log::error('Failed to save user avatar image', ['exception' => $e]);
         }
     }
 
@@ -49,7 +49,7 @@ class UserAvatars
             $user->avatar()->associate($avatar);
             $user->save();
         } catch (Exception $e) {
-            Log::error('Failed to save user avatar image');
+            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;

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);
+    }
 }