From ecb83d0279f0eb3df5217b945980a2485c76c1cb Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Fri, 20 Aug 2021 14:35:39 +1000 Subject: [PATCH] FIX: Adding debugging and fixing media-optimization-worker issues (#14099) When we encountered an error with the media-optimization-worker, we stopped the worker, which made it so further messages were not received when optimizing images in parallel. Removed this based on an option. Also added more debugging lines to help track down issues. --- ...ter-media-optimization-upload-processor.js | 4 +- .../app/lib/uppy-media-optimization-plugin.js | 2 +- .../app/services/media-optimization-worker.js | 40 ++++++++++++------- 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/app/assets/javascripts/discourse/app/initializers/register-media-optimization-upload-processor.js b/app/assets/javascripts/discourse/app/initializers/register-media-optimization-upload-processor.js index 3b6a9758260..1a2e4276f6e 100644 --- a/app/assets/javascripts/discourse/app/initializers/register-media-optimization-upload-processor.js +++ b/app/assets/javascripts/discourse/app/initializers/register-media-optimization-upload-processor.js @@ -9,10 +9,10 @@ export default { addComposerUploadProcessor( { action: "optimizeJPEG" }, { - optimizeJPEG: (data) => + optimizeJPEG: (data, opts) => container .lookup("service:media-optimization-worker") - .optimizeImage(data), + .optimizeImage(data, opts), } ); } diff --git a/app/assets/javascripts/discourse/app/lib/uppy-media-optimization-plugin.js b/app/assets/javascripts/discourse/app/lib/uppy-media-optimization-plugin.js index a13b90a371d..3ba4cc8fca0 100644 --- a/app/assets/javascripts/discourse/app/lib/uppy-media-optimization-plugin.js +++ b/app/assets/javascripts/discourse/app/lib/uppy-media-optimization-plugin.js @@ -23,7 +23,7 @@ export default class UppyMediaOptimization extends Plugin { this.uppy.emit("preprocess-progress", this.pluginClass, file); - return this.optimizeFn(file) + return this.optimizeFn(file, { stopWorkerOnError: !this.runParallel }) .then((optimizedFile) => { if (!optimizedFile) { warn("Nothing happened, possible error or other restriction.", { 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 ba01edb7495..1cac1783573 100644 --- a/app/assets/javascripts/discourse/app/services/media-optimization-worker.js +++ b/app/assets/javascripts/discourse/app/services/media-optimization-worker.js @@ -12,18 +12,20 @@ export default class MediaOptimizationWorkerService extends Service { promiseResolvers = null; startWorker() { + this.logIfDebug("Starting media-optimization-worker"); this.worker = new Worker(this.workerUrl); // TODO come up with a workaround for FF that lacks type: module support } stopWorker() { + this.logIfDebug("Stopping media-optimization-worker..."); this.worker.terminate(); this.worker = null; } - ensureAvailiableWorker(usingUppy) { + ensureAvailiableWorker() { if (!this.worker) { this.startWorker(); - this.registerMessageHandler(usingUppy); + this.registerMessageHandler(); this.appEvents.on("composer:closed", this, "stopWorker"); } } @@ -35,22 +37,25 @@ export default class MediaOptimizationWorkerService extends Service { } } - optimizeImage(data) { - const usingUppy = data.id && data.id.includes("uppy"); + 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 = usingUppy ? data : data.files[data.index]; + let file = this.usingUppy ? data : data.files[data.index]; if (!/(\.|\/)(jpe?g|png|webp)$/i.test(file.type)) { - return usingUppy ? Promise.resolve() : data; + return this.usingUppy ? Promise.resolve() : data; } if ( file.size < this.siteSettings .composer_media_optimization_image_bytes_optimization_threshold ) { - return usingUppy ? Promise.resolve() : data; + return this.usingUppy ? Promise.resolve() : data; } - this.ensureAvailiableWorker(usingUppy); + this.ensureAvailiableWorker(); return new Promise(async (resolve) => { this.logIfDebug(`Transforming ${file.name}`); @@ -59,14 +64,14 @@ export default class MediaOptimizationWorkerService extends Service { let imageData; try { - if (usingUppy) { + if (this.usingUppy) { imageData = await fileToImageData(file.data); } else { imageData = await fileToImageData(file); } } catch (error) { this.logIfDebug(error); - return usingUppy ? resolve() : resolve(data); + return this.usingUppy ? resolve() : resolve(data); } this.worker.postMessage( @@ -108,7 +113,7 @@ export default class MediaOptimizationWorkerService extends Service { }); } - registerMessageHandler(usingUppy) { + registerMessageHandler() { this.worker.onmessage = (e) => { switch (e.data.type) { case "file": @@ -119,7 +124,7 @@ export default class MediaOptimizationWorkerService extends Service { `Finished optimization of ${optimizedFile.name} new size: ${optimizedFile.size}.` ); - if (usingUppy) { + if (this.usingUppy) { this.promiseResolvers[optimizedFile.name](optimizedFile); } else { let data = this.currentComposerUploadData; @@ -129,8 +134,15 @@ export default class MediaOptimizationWorkerService extends Service { break; case "error": - this.stopWorker(); - if (usingUppy) { + this.logIfDebug( + `Handling error message from image optimization for ${e.data.fileName}.` + ); + + if (this.stopWorkerOnError) { + this.stopWorker(); + } + + if (this.usingUppy) { this.promiseResolvers[e.data.fileName](); } else { this.promiseResolvers[e.data.fileName](