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.

diff --git a/app/Http/Controllers/Images/DrawioImageController.php b/app/Http/Controllers/Images/DrawioImageController.php
index cab1c925e414855a3a8be13b08e2c825dc101360..ce857cf52f07b59b238709cfb47c0fae0efb2c6c 100644 (file)
--- a/app/Http/Controllers/Images/DrawioImageController.php
+++ b/app/Http/Controllers/Images/DrawioImageController.php
@@ -66,14 +66,19 @@ class DrawioImageController extends Controller
      */
     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([

diff --git a/resources/js/services/drawio.js b/resources/js/services/drawio.js
index dfca832117f28b8c5ac12014a729bc0fc0610483..67ac4cc0ecb31fd51f52c785990149a17d6da5f4 100644 (file)
--- a/resources/js/services/drawio.js
+++ b/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};
\ No newline at end of file

diff --git a/resources/js/services/events.js b/resources/js/services/events.js
index 6668014e7b6913ca4fbee93fe11f69307ada3349..d2dacfa7b7c3f9f50ac21a8ae6cd1e4651472f17 100644 (file)
--- a/resources/js/services/events.js
+++ b/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,6 +54,17 @@ 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,
@@ -63,4 +72,5 @@ export default {
     success: (msg) => emit('success', msg),
     error: (msg) => emit('error', msg),
     showValidationErrors,
+    showResponseError,
 }
\ No newline at end of file

diff --git a/resources/js/services/http.js b/resources/js/services/http.js
index 00865e69bd8ff83698864ed7c6d0dd0042ef465f..211ea0c92b75ceaca9fa869527e8563758bb21b6 100644 (file)
--- a/resources/js/services/http.js
+++ b/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,
 };
\ No newline at end of file

diff --git a/resources/js/wysiwyg/plugin-drawio.js b/resources/js/wysiwyg/plugin-drawio.js
index ad7e09f95fabbcf6f79a0b0f19a08e2d353df77a..9f4a065adf867561f52e7ddc0e7f79202b13c275 100644 (file)
--- a/resources/js/wysiwyg/plugin-drawio.js
+++ b/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);
 }
 

diff --git a/resources/lang/en/errors.php b/resources/lang/en/errors.php
index 52f96cbe741852c60b45ec62cd9da6726e7f1bf4..703d0edbef0635a4963c2eb7e294f0d7c65bf0b1 100644 (file)
--- a/resources/lang/en/errors.php
+++ b/resources/lang/en/errors.php
@@ -45,9 +45,12 @@ return [
     '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',

diff --git a/tests/Uploads/DrawioTest.php b/tests/Uploads/DrawioTest.php
index 080f05d7402f8971db9eff17d0d86459b03124bb..3c208f54d713d43a4c4affca8302e4fe907cfe90 100644 (file)
--- a/tests/Uploads/DrawioTest.php
+++ b/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 @@ class DrawioTest extends TestCase
         ]);
     }
 
+    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 @@ class DrawioTest extends TestCase
     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 @@ class DrawioTest extends TestCase
     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'));