From 995c514336241633c0783f1c917f0410659b0f98 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Tue, 21 Dec 2021 09:00:19 +1000 Subject: [PATCH] DEV: Handle CORS and other fetch failures for media-optimization-worker (#15364) Occasionally there will be a misconfigured CORS rule or a different network failure when loading one of the media optimization WASM scripts. This commit handles load failures and sends a new installFailed message from the service worker, so that we don't error and hold up the rest of the uploads if this occurs; the worker will just not process anything and will keep trying to install itself with subsequent uploads until it succeeds. This commit also removes the redundant useUppy variable in the worker this should have been removed a while ago in f70e6c302fddd38595a1fc90e24964a2cb0716bb --- .../app/services/media-optimization-worker.js | 50 ++++++++----------- .../javascripts/media-optimization-worker.js | 8 ++- 2 files changed, 28 insertions(+), 30 deletions(-) diff --git a/app/assets/javascripts/discourse/app/services/media-optimization-worker.js b/app/assets/javascripts/discourse/app/services/media-optimization-worker.js index 4feec51f37e..3699ffb5ae0 100644 --- a/app/assets/javascripts/discourse/app/services/media-optimization-worker.js +++ b/app/assets/javascripts/discourse/app/services/media-optimization-worker.js @@ -28,22 +28,21 @@ export default class MediaOptimizationWorkerService extends Service { promiseResolvers = null; async optimizeImage(data, opts = {}) { - this.usingUppy = data.id && data.id.includes("uppy"); this.promiseResolvers = this.promiseResolvers || {}; this.stopWorkerOnError = opts.hasOwnProperty("stopWorkerOnError") ? opts.stopWorkerOnError : true; - let file = this.usingUppy ? data : data.files[data.index]; + let file = data; if (!/(\.|\/)(jpe?g|png|webp)$/i.test(file.type)) { - return this.usingUppy ? Promise.resolve() : data; + return Promise.resolve(); } if ( file.size < this.siteSettings .composer_media_optimization_image_bytes_optimization_threshold ) { - return this.usingUppy ? Promise.resolve() : data; + return Promise.resolve(); } await this.ensureAvailiableWorker(); @@ -51,18 +50,14 @@ export default class MediaOptimizationWorkerService extends Service { this.logIfDebug(`Transforming ${file.name}`); this.currentComposerUploadData = data; - this.promiseResolvers[this.usingUppy ? file.id : file.name] = resolve; + this.promiseResolvers[file.id] = resolve; let imageData; try { - if (this.usingUppy) { - imageData = await fileToImageData(file.data); - } else { - imageData = await fileToImageData(file); - } + imageData = await fileToImageData(file.data); } catch (error) { this.logIfDebug(error); - return this.usingUppy ? resolve() : resolve(data); + return resolve(); } this.worker.postMessage( @@ -104,8 +99,9 @@ export default class MediaOptimizationWorkerService extends Service { } async install() { - this.installPromise = new Promise((resolve) => { + this.installPromise = new Promise((resolve, reject) => { this.afterInstalled = resolve; + this.failedInstall = reject; this.logIfDebug("Installing worker."); this.startWorker(); this.registerMessageHandler(); @@ -152,13 +148,7 @@ export default class MediaOptimizationWorkerService extends Service { `Finished optimization of ${optimizedFile.name} new size: ${optimizedFile.size}.` ); - if (this.usingUppy) { - this.promiseResolvers[e.data.fileId](optimizedFile); - } else { - let data = this.currentComposerUploadData; - data.files[data.index] = optimizedFile; - this.promiseResolvers[optimizedFile.name](data); - } + this.promiseResolvers[e.data.fileId](optimizedFile); break; case "error": @@ -170,20 +160,18 @@ export default class MediaOptimizationWorkerService extends Service { this.stopWorker(); } - if (this.usingUppy) { - this.promiseResolvers[e.data.fileId](); - } else { - this.promiseResolvers[e.data.fileName]( - this.currentComposerUploadData - ); - } + this.promiseResolvers[e.data.fileId](); break; case "installed": this.logIfDebug("Worker installed."); this.workerInstalled = true; this.afterInstalled(); - this.afterInstalled = null; - this.installPromise = null; + this.cleanupInstallPromises(); + break; + case "installFailed": + this.logIfDebug("Worker failed to install."); + this.failedInstall(e.data.errorMessage); + this.cleanupInstallPromises(); break; default: this.logIfDebug(`Sorry, we are out of ${e}.`); @@ -191,6 +179,12 @@ export default class MediaOptimizationWorkerService extends Service { }; } + cleanupInstallPromises() { + this.afterInstalled = null; + this.failedInstall = null; + this.installPromise = null; + } + logIfDebug(message) { if (this.siteSettings.composer_media_optimization_debug_mode) { // eslint-disable-next-line no-console diff --git a/public/javascripts/media-optimization-worker.js b/public/javascripts/media-optimization-worker.js index b3fd8f01271..c34c3c5c138 100644 --- a/public/javascripts/media-optimization-worker.js +++ b/public/javascripts/media-optimization-worker.js @@ -148,8 +148,12 @@ onmessage = async function (e) { } break; case "install": - await loadLibs(e.data.settings); - postMessage({ type: "installed" }); + try { + await loadLibs(e.data.settings); + postMessage({ type: "installed" }); + } catch (error) { + postMessage({ type: "installFailed", errorMessage: error.message }); + } break; default: logIfDebug(`Sorry, we are out of ${e}.`);