From cf0b9d93b0c09773c749ef6f8da6810e5f86689f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20=27rysiek=27=20Wo=C5=BAniak?= Date: Wed, 6 Mar 2024 13:47:10 +0000 Subject: [PATCH] further improved error handling, fixing some corner cases (ref. #36) --- .../service-worker/service-worker.test.js | 2 +- service-worker.js | 34 +++++++++++++------ 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/__tests__/service-worker/service-worker.test.js b/__tests__/service-worker/service-worker.test.js index 93b1f49..5a1d814 100644 --- a/__tests__/service-worker/service-worker.test.js +++ b/__tests__/service-worker/service-worker.test.js @@ -2705,7 +2705,7 @@ describe('service-worker', async () => { let response = await fetch_event.waitForResponse() await new Promise(resolve => setTimeout(resolve, (self.LibResilientConfig.stillLoadingTimeout + 50))) - assertEquals((await response.text()).slice(0, 58), 'Still loading...') + assertEquals((await response.text()).slice(0, 55), 'Still loading') }) it("should not use the still-loading screen when handling a regular request, even if a stashing plugin is configured and enabled and stillLoadingTimeout set to a positive integer", async () => { diff --git a/service-worker.js b/service-worker.js index 2285ed9..4b49c7e 100644 --- a/service-worker.js +++ b/service-worker.js @@ -274,12 +274,12 @@ let cacheConfigJSON = async (configURL, cresponse, use_source) => { * cresponse - the Response object to work with */ let getConfigJSON = async (cresponse) => { - if (cresponse == undefined) { - self.log('service-worker', 'config.json response is undefined') + if ( (cresponse === undefined) || (cresponse === null) || (typeof cresponse !== 'object') || ! ("status" in cresponse) || ! ("statusText" in cresponse) ) { + self.log('service-worker', 'config.json response is undefined or invalid') return false; } if (cresponse.status != 200) { - self.log('service-worker', `config.json response status is not 200: ${cdata.status} ${cdata.statusText})`) + self.log('service-worker', `config.json response status is not 200: ${cresponse.status} ${cresponse.statusText})`) return false; } // cloning the response before applying json() @@ -687,6 +687,9 @@ let initServiceWorker = async () => { cacheConfigJSON(configURL, cresponse, "v1") } }) + .catch((e)=>{ + self.log('service-worker', `stale config.json fetch failed.`) + }) } } catch(e) { @@ -938,7 +941,7 @@ let LibResilientResourceInfo = class { // push the error info, along with method that generated it, onto the error stack this.errors.push([this.values.method, data.error]) // response? - if ("statusText" in data.error) { + if ( (typeof data.error === 'object') && ("statusText" in data.error) ) { msgdata.error = `HTTP status: ${data.error.status} ${data.error.statusText}` // nope, exception } else { @@ -1026,10 +1029,19 @@ let initFromRequest = (req) => { * reqInfo - instance of LibResilientResourceInfo */ let libresilientFetch = (plugin, url, init, reqInfo) => { + // we really need to catch any exceptions here // otherwise other plugins will not run! // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/catch#gotchas_when_throwing_errors try { + + // do we actually have a plugin to work with? + if ( (! plugin) || (typeof plugin !== "object") || ! ("name" in plugin) || ! ("fetch" in plugin) ) { + return Promise.reject( + new Error("Plugin is not valid.") + ) + } + // status of the plugin reqInfo.update({ method: (plugin && "name" in plugin) ? plugin.name : "unknown", @@ -1183,7 +1195,7 @@ let getResourceThroughLibResilient = (url, init, clientId, useStashed=true, doSt // if it's a stashing plugin... if (typeof plugin.stash === 'function') { // we obviously do not want to stash - self.log('service-worker', 'Not stashing, since resource is already retrieved by a stashing plugin:', url); + self.log('service-worker', 'not stashing, since resource is already retrieved by a stashing plugin:', url); // since we got the data from a stashing plugin, // let's run the rest of plugins in the background to check if we can get a fresher resource // and stash it in cache for later use @@ -1193,7 +1205,7 @@ let getResourceThroughLibResilient = (url, init, clientId, useStashed=true, doSt try { getResourceThroughLibResilient(url, init, clientId, false, true, response.clone()) .catch((e)=>{ - self.log('service-worker', 'background no-stashed fetch failed for:', url); + self.log('service-worker', 'background no-stashed fetch failed for:', url, `\n+-- error: ${e}`); }) } catch(e) { self.log('service-worker', 'background no-stashed fetch failed for:', url, `\n+-- error: ${e}`); @@ -1267,8 +1279,7 @@ let getResourceThroughLibResilient = (url, init, clientId, useStashed=true, doSt }) // a final catch... in case all plugins fail .catch((err)=>{ - self.log('service-worker', "all plugins failed: ", err, - '\n+-- URL : ' + url) + self.log('service-worker', `all plugins failed for:\n+-- url : ${url}`) // cleanup reqInfo.update({ @@ -1283,11 +1294,14 @@ let getResourceThroughLibResilient = (url, init, clientId, useStashed=true, doSt reqInfo .errors .reduce((acc, cur)=>{ + if ( (typeof cur[0] !== "string") || (cur[0].length === 0) ) { + cur[0] = '' + } return acc + ' ' + cur.join(': ') + '\n' }, '')}`) - // rethrow - throw err + // return the error wrapped in a rejected promise + return Promise.reject(err) }) }