Added better drawing load failure handling

Failure of loading drawings will now close the drawing view and show an
error message, hinting at file or permission issues, instead of leaving
the user facing a continuosly loading interface.

Adds test to cover.

This also updates errors from our HTTP service to be wrapped in a custom
error type for better identification and so the error is an actual
javascript error. Should be object compatible.

Related to #3955.

Author: ssddanbrown

app/Http/Controllers/Images/DrawioImageController.php

@@ -66,14 +66,19 @@ public function create(Request $request)
      */
     public function getAsBase64($id)
     {
-        $image = $this->imageRepo->getById($id);
-        if (is_null($image) || $image->type !== 'drawio' || !userCan('page-view', $image->getPage())) {
-            return $this->jsonError('Image data could not be found');
+        try {
+            $image = $this->imageRepo->getById($id);
+        } catch (Exception $exception) {
+            return $this->jsonError(trans('errors.drawing_data_not_found'), 404);
+        }
+
+        if ($image->type !== 'drawio' || !userCan('page-view', $image->getPage())) {
+            return $this->jsonError(trans('errors.drawing_data_not_found'), 404);
         }
 
         $imageData = $this->imageRepo->getImageData($image);
         if (is_null($imageData)) {
-            return $this->jsonError('Image data could not be found');
+            return $this->jsonError(trans('errors.drawing_data_not_found'), 404);
         }
 
         return response()->json([

resources/js/services/drawio.js

@@ -95,8 +95,16 @@ async function upload(imageData, pageUploadedToId) {
  * @returns {Promise<string>}
  */
 async function load(drawingId) {
-    const resp = await window.$http.get(window.baseUrl(`/images/drawio/base64/${drawingId}`));
-    return `data:image/png;base64,${resp.data.content}`;
+    try {
+        const resp = await window.$http.get(window.baseUrl(`/images/drawio/base64/${drawingId}`));
+        return `data:image/png;base64,${resp.data.content}`;
+    } catch (error) {
+        if (error instanceof window.$http.HttpError) {
+            window.$events.showResponseError(error);
+        }
+        close();
+        throw error;
+    }
 }
 
 export default {show, close, upload, load};

resources/js/services/events.js

@@ -43,10 +43,8 @@ function emitPublic(targetElement, eventName, eventData) {
 }
 
 /**
- * Notify of a http error.
- * Check for standard scenarios such as validation errors and
- * formats an error notification accordingly.
- * @param {Error} error
+ * Notify of standard server-provided validation errors.
+ * @param {Object} error
  */
 function showValidationErrors(error) {
     if (!error.status) return;
@@ -56,11 +54,23 @@ function showValidationErrors(error) {
     }
 }
 
+/**
+ * Notify standard server-provided error messages.
+ * @param {Object} error
+ */
+function showResponseError(error) {
+    if (!error.status) return;
+    if (error.status >= 400 && error.data && error.data.message) {
+        emit('error', error.data.message);
+    }
+}
+
 export default {
     emit,
     emitPublic,
     listen,
     success: (msg) => emit('success', msg),
     error: (msg) => emit('error', msg),
     showValidationErrors,
+    showResponseError,
 }

resources/js/services/http.js

@@ -132,7 +132,7 @@ async function request(url, options = {}) {
     };
 
     if (!response.ok) {
-        throw returnData;
+        throw new HttpError(response, content);
     }
 
     return returnData;
@@ -159,10 +159,24 @@ async function getResponseContent(response) {
     return await response.text();
 }
 
+class HttpError extends Error {
+    constructor(response, content) {
+        super(response.statusText);
+        this.data = content;
+        this.headers = response.headers;
+        this.redirected = response.redirected;
+        this.status = response.status;
+        this.statusText = response.statusText;
+        this.url = response.url;
+        this.original = response;
+    }
+}
+
 export default {
     get: get,
     post: post,
     put: put,
     patch: patch,
     delete: performDelete,
+    HttpError: HttpError,
 };

resources/js/wysiwyg/plugin-drawio.js

@@ -89,7 +89,7 @@ function drawingInit() {
         return Promise.resolve('');
     }
 
-    let drawingId = currentNode.getAttribute('drawio-diagram');
+    const drawingId = currentNode.getAttribute('drawio-diagram');
     return DrawIO.load(drawingId);
 }
 

resources/lang/en/errors.php

@@ -45,9 +45,12 @@
     'cannot_create_thumbs' => 'The server cannot create thumbnails. Please check you have the GD PHP extension installed.',
     'server_upload_limit' => 'The server does not allow uploads of this size. Please try a smaller file size.',
     'uploaded'  => 'The server does not allow uploads of this size. Please try a smaller file size.',
+    'file_upload_timeout' => 'The file upload has timed out.',
+
+    // Drawing & Images
     'image_upload_error' => 'An error occurred uploading the image',
     'image_upload_type_error' => 'The image type being uploaded is invalid',
-    'file_upload_timeout' => 'The file upload has timed out.',
+    'drawing_data_not_found' => 'Drawing data could not be loaded. The drawing file might no longer exist or you may not have permission to access it.',
 
     // Attachments
     'attachment_not_found' => 'Attachment not found',

tests/Uploads/DrawioTest.php

@@ -12,12 +12,13 @@ class DrawioTest extends TestCase
 
     public function test_get_image_as_base64()
     {
-        $page = Page::first();
+        $page = $this->entities->page();
         $this->asAdmin();
         $imageName = 'first-image.png';
 
         $this->uploadImage($imageName, $page->id);
-        $image = Image::first();
+        /** @var Image $image */
+        $image = Image::query()->first();
         $image->type = 'drawio';
         $image->save();
 
@@ -27,9 +28,29 @@ public function test_get_image_as_base64()
         ]);
     }
 
+    public function test_non_accessible_image_returns_404_error_and_message()
+    {
+        $page = $this->entities->page();
+        $this->asEditor();
+        $imageName = 'non-accessible-image.png';
+
+        $this->uploadImage($imageName, $page->id);
+        /** @var Image $image */
+        $image = Image::query()->first();
+        $image->type = 'drawio';
+        $image->save();
+        $this->permissions->disableEntityInheritedPermissions($page);
+
+        $imageGet = $this->getJson("/images/drawio/base64/{$image->id}");
+        $imageGet->assertNotFound();
+        $imageGet->assertJson([
+            'message' => 'Drawing data could not be loaded. The drawing file might no longer exist or you may not have permission to access it.',
+        ]);
+    }
+
     public function test_drawing_base64_upload()
     {
-        $page = Page::first();
+        $page = $this->entities->page();
         $editor = $this->users->editor();
         $this->actingAs($editor);
 
@@ -57,7 +78,7 @@ public function test_drawing_base64_upload()
     public function test_drawio_url_can_be_configured()
     {
         config()->set('services.drawio', 'http://cats.com?dog=tree');
-        $page = Page::first();
+        $page = $this->entities->page();
         $editor = $this->users->editor();
 
         $resp = $this->actingAs($editor)->get($page->getUrl('/edit'));
@@ -67,7 +88,7 @@ public function test_drawio_url_can_be_configured()
     public function test_drawio_url_can_be_disabled()
     {
         config()->set('services.drawio', true);
-        $page = Page::first();
+        $page = $this->entities->page();
         $editor = $this->users->editor();
 
         $resp = $this->actingAs($editor)->get($page->getUrl('/edit'));