From 48df8725d82cac821ee935498c811509c96e63c2 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Thu, 26 Jan 2023 12:16:23 +0000
Subject: [PATCH] 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.
---
.../Images/DrawioImageController.php | 13 +++++---
resources/js/services/drawio.js | 12 +++++--
resources/js/services/events.js | 18 ++++++++---
resources/js/services/http.js | 16 +++++++++-
resources/js/wysiwyg/plugin-drawio.js | 2 +-
resources/lang/en/errors.php | 5 ++-
tests/Uploads/DrawioTest.php | 31 ++++++++++++++++---
7 files changed, 79 insertions(+), 18 deletions(-)
diff --git a/app/Http/Controllers/Images/DrawioImageController.php b/app/Http/Controllers/Images/DrawioImageController.php
index cab1c925e..ce857cf52 100644
--- 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 dfca83211..67ac4cc0e 100644
--- 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 6668014e7..d2dacfa7b 100644
--- 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 00865e69b..211ea0c92 100644
--- 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 ad7e09f95..9f4a065ad 100644
--- 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 52f96cbe7..703d0edbe 100644
--- 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 080f05d74..3c208f54d 100644
--- 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'));