From 2406bca941ce92eced975a4a14de56c60eee5e71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20=27rysiek=27=20Wo=C5=BAniak?= Date: Sun, 25 Feb 2024 18:36:53 +0000 Subject: [PATCH 01/17] error plugin can now into headers --- plugins/error/index.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/plugins/error/index.js b/plugins/error/index.js index 74fe393..6b62055 100644 --- a/plugins/error/index.js +++ b/plugins/error/index.js @@ -23,6 +23,8 @@ type: "http", // only valid if type: http code: 500, + // valid only if type: http + headers: {}, // valid either way message: "Internal server error" } @@ -43,14 +45,17 @@ LR.log(pluginName, `erroring out for: ${url} — HTTP error`) // I guess we want a HTTP error then - var responseInit = { status: config.code, statusText: config.message, - headers: {}, + headers: config.headers, url: url }; - responseInit.headers['Content-Type'] = "text/plain" + + // we need some content type here + if (responseInit.headers['Content-Type'] === undefined) { + responseInit.headers['Content-Type'] = "text/plain" + } let blob = new Blob( [config.message], From f5facd288a13636e2c37b4cfddd66c82f3574776 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20=27rysiek=27=20Wo=C5=BAniak?= Date: Wed, 28 Feb 2024 02:57:08 +0000 Subject: [PATCH 02/17] clarifying comment in alt-fetch plugin code --- plugins/alt-fetch/index.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/plugins/alt-fetch/index.js b/plugins/alt-fetch/index.js index 124217a..61eba29 100644 --- a/plugins/alt-fetch/index.js +++ b/plugins/alt-fetch/index.js @@ -105,6 +105,10 @@ )) .then((response) => { // 4xx? 5xx? that's a paddlin' + // NOTICE: normally 4xx errors are returned to the client by other plugins, + // NOTICE: but here we are relying on multiple alternative endpoints; + // NOTICE: so, we want to maximize the chance that we get *something* useful + // TODO: shouldn't this reject() instead if (response.status >= 400) { // throw an Error to fall back to other plugins: throw new Error('HTTP Error: ' + response.status + ' ' + response.statusText); From 5bae7cace6c2dd4bfd623fd98514ea84cdfbde6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20=27rysiek=27=20Wo=C5=BAniak?= Date: Wed, 28 Feb 2024 02:58:28 +0000 Subject: [PATCH 03/17] minor improvements, including fetch plugin now returning 4xx errors directly as Response objects (ref. #36) --- plugins/delay/index.js | 5 +++++ plugins/fetch/index.js | 7 ++++--- service-worker.js | 2 ++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/plugins/delay/index.js b/plugins/delay/index.js index aa36553..c5632d6 100644 --- a/plugins/delay/index.js +++ b/plugins/delay/index.js @@ -35,6 +35,11 @@ // merge the defaults with settings from init let config = {...defaultConfig, ...init} + // reality check: if no wrapped plugin configured, or more than one, complain + if (config.uses.length != 1) { + throw new Error(`Expected exactly one plugin to wrap, but ${config.uses.length} configured.`) + } + /** * getting content using regular HTTP(S) fetch() */ diff --git a/plugins/fetch/index.js b/plugins/fetch/index.js index d7fe153..d240e6f 100644 --- a/plugins/fetch/index.js +++ b/plugins/fetch/index.js @@ -32,13 +32,14 @@ // run built-in regular fetch() return fetch(url, init) .then((response) => { - // 4xx? 5xx? that's a paddlin' - if (response.status >= 400) { + // 5xx? that's a paddlin' + // we do want to pass 3xx and 4xx on back to the client though! + if (response.status >= 500) { // throw an Error to fall back to LibResilient: throw new Error('HTTP Error: ' + response.status + ' ' + response.statusText); } // all good, it seems - LR.log(pluginName, `fetched successfully: ${response.url}`); + LR.log(pluginName, `fetched:\n+-- url: ${response.url}\n+-- http status: ${response.status} (${response.statusText})`); // we need to create a new Response object // with all the headers added explicitly, diff --git a/service-worker.js b/service-worker.js index c7f010f..293cf00 100644 --- a/service-worker.js +++ b/service-worker.js @@ -820,6 +820,8 @@ let LibResilientClient = class { constructor(clientId) { + self.log('service-worker', `new client: ${clientId}`) + // we often get the clientId long before // we are able to get a valid client out of it // From 17d1aee6dbaea2daf3471b7ecf49067ef0edd815 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20=27rysiek=27=20Wo=C5=BAniak?= Date: Wed, 28 Feb 2024 05:39:52 +0000 Subject: [PATCH 04/17] this.client is not used anymore in LibResilientResourceInfo --- service-worker.js | 1 - 1 file changed, 1 deletion(-) diff --git a/service-worker.js b/service-worker.js index 293cf00..0e3c82c 100644 --- a/service-worker.js +++ b/service-worker.js @@ -888,7 +888,6 @@ let LibResilientResourceInfo = class { state: null, // can be "failed", "success", "running" serviceWorker: 'COMMIT_UNKNOWN' // this will be replaced by commit sha in CI/CD; read-only } - this.client = null; // queued messages for when we have a client available this.messageQueue = [] From 81dfaab7c51c22b5430613295698808c38e1a698 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20=27rysiek=27=20Wo=C5=BAniak?= Date: Wed, 28 Feb 2024 06:33:37 +0000 Subject: [PATCH 05/17] LibResilientResourceInfo can now hold errors for all plugins (ref. #36) --- service-worker.js | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/service-worker.js b/service-worker.js index 0e3c82c..35f98bc 100644 --- a/service-worker.js +++ b/service-worker.js @@ -883,12 +883,14 @@ let LibResilientResourceInfo = class { this.values = { url: '', // read only after initialization clientId: null, // the client on whose behalf that request is being processed - lastError: null, // error from the previous plugin (for state:running) or the last emitted error (for state:failed or state:success) method: null, // name of the current plugin (in case of state:running) or last plugin (for state:failed or state:success) state: null, // can be "failed", "success", "running" serviceWorker: 'COMMIT_UNKNOWN' // this will be replaced by commit sha in CI/CD; read-only } + // errors from the plugins, contains tuples: [plugin-name, exception-or-response-object] + this.errors = [] + // queued messages for when we have a client available this.messageQueue = [] @@ -915,11 +917,11 @@ let LibResilientResourceInfo = class { var msg = 'Updated LibResilientResourceInfo for: ' + this.values.url // was there a change? if not, no need to postMessage var changed = false - // update the properties that are read-write + // update simple read-write properties Object .keys(data) .filter((k)=>{ - return ['lastError', 'method', 'state'].includes(k) + return ['method', 'state'].includes(k) }) .forEach((k)=>{ msg += '\n+-- ' + k + ': ' + data[k] @@ -929,23 +931,31 @@ let LibResilientResourceInfo = class { } this.values[k] = data[k] }) + // start preparing the data to postMessage() over to the client + let msgdata = {...this.values} + // handle any error related info + if ('error' in data) { + // 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) { + msgdata.error = `HTTP status: ${data.error.status} ${data.error.statusText}` + // nope, exception + } else { + msgdata.error = data.error.toString() + } + changed = true + } self.log('service-worker', msg) // send the message to the client if (changed) { postMessage( this.values.clientId, - {...this.values} + msgdata ); } } - /** - * lastError property - */ - get lastError() { - return this.values.lastError - } - /** * method property */ @@ -1129,7 +1139,7 @@ let getResourceThroughLibResilient = (url, init, clientId, useStashed=true, doSt '\n+-- error : ' + error.toString()) // save info in reqInfo -- status of the previous method reqInfo.update({ - lastError: error.toString() + error: error }) return libresilientFetch(currentPlugin, url, init, reqInfo) }) @@ -1146,7 +1156,6 @@ let getResourceThroughLibResilient = (url, init, clientId, useStashed=true, doSt // record the success reqInfo.update({ - lastError: null, state:"success" }) @@ -1246,7 +1255,7 @@ let getResourceThroughLibResilient = (url, init, clientId, useStashed=true, doSt // cleanup reqInfo.update({ state: "failed", - lastError: err.toString() + error: err }) decrementActiveFetches(clientId) // rethrow @@ -1469,7 +1478,7 @@ self.addEventListener('fetch', async event => { header.innerHTML = "Loading failed." text.innerHTML = "We're sorry, we were unable to load this page." } - if ( ( 'lastError' in event.data ) && ( typeof event.data.lastError === 'string' ) ) { + if ( ( 'error' in event.data ) && ( typeof event.data.error === 'object' ) ) { attempts += 1; status.innerHTML = attempts; } From 0932bf9b9f6ab7f3f6940b8166c9d316326ccbec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20=27rysiek=27=20Wo=C5=BAniak?= Date: Wed, 28 Feb 2024 06:47:02 +0000 Subject: [PATCH 06/17] exceptions thrown in plugins are now properly caught in libresilientFetch (ref. #36) --- service-worker.js | 89 +++++++++++++++++++++++++---------------------- 1 file changed, 48 insertions(+), 41 deletions(-) diff --git a/service-worker.js b/service-worker.js index 35f98bc..c4fccb1 100644 --- a/service-worker.js +++ b/service-worker.js @@ -1026,48 +1026,55 @@ let initFromRequest = (req) => { * reqInfo - instance of LibResilientResourceInfo */ let libresilientFetch = (plugin, url, init, reqInfo) => { - // status of the plugin - reqInfo.update({ - method: (plugin && "name" in plugin) ? plugin.name : "unknown", - state: "running" - }) - - // log stuff - self.log('service-worker', "LibResilient Service Worker handling URL:", url, - '\n+-- init:', Object.getOwnPropertyNames(init).map(p=>`\n - ${p}: ${init[p]}`).join(''), - '\n+-- using method(s):', plugin.name - ) - - // starting the fetch... - // if it errors out immediately, at least we don't have to deal - // with a dangling promise timeout, set up below - let fetch_promise = plugin.fetch(url, init) - - let timeout_promise, timeout_id - [timeout_promise, timeout_id] = promiseTimeout( - self.LibResilientConfig.defaultPluginTimeout, - false, - `LibResilient request using ${plugin.name} timed out after ${self.LibResilientConfig.defaultPluginTimeout}ms.` - ) - - // making sure there are no dangling promises etc - // - // this has to happen asynchronously - fetch_promise - // make sure the timeout is cancelled as soon as the promise resolves - // we do not want any dangling promises/timeouts after all! - .finally(()=>{ - clearTimeout(timeout_id) + // 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 { + // status of the plugin + reqInfo.update({ + method: (plugin && "name" in plugin) ? plugin.name : "unknown", + state: "running" }) - // no-op to make sure we don't end up with dangling rejected premises - .catch((e)=>{}) - - // race the plugin(s) vs. the timeout - return Promise - .race([ - fetch_promise, - timeout_promise - ]); + + // log stuff + self.log('service-worker', "LibResilient Service Worker handling URL:", url, + '\n+-- init:', Object.getOwnPropertyNames(init).map(p=>`\n - ${p}: ${init[p]}`).join(''), + '\n+-- using method(s):', plugin.name + ) + + // starting the fetch... + // if it errors out immediately, at least we don't have to deal + // with a dangling promise timeout, set up below + let fetch_promise = plugin.fetch(url, init) + + let timeout_promise, timeout_id + [timeout_promise, timeout_id] = promiseTimeout( + self.LibResilientConfig.defaultPluginTimeout, + false, + `LibResilient request using ${plugin.name} timed out after ${self.LibResilientConfig.defaultPluginTimeout}ms.` + ) + + // making sure there are no dangling promises etc + // + // this has to happen asynchronously + fetch_promise + // make sure the timeout is cancelled as soon as the promise resolves + // we do not want any dangling promises/timeouts after all! + .finally(()=>{ + clearTimeout(timeout_id) + }) + // no-op to make sure we don't end up with dangling rejected premises + .catch((e)=>{}) + + // race the plugin(s) vs. the timeout + return Promise + .race([ + fetch_promise, + timeout_promise + ]); + } catch(e) { + return Promise.reject(e) + } } From 18e436568a6af33bad7ab2243c9a73ceccb53b23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20=27rysiek=27=20Wo=C5=BAniak?= Date: Wed, 28 Feb 2024 07:01:44 +0000 Subject: [PATCH 07/17] the still-loading screen now displays plugin errors (re. #36) --- service-worker.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/service-worker.js b/service-worker.js index c4fccb1..06e560f 100644 --- a/service-worker.js +++ b/service-worker.js @@ -1462,16 +1462,26 @@ self.addEventListener('fetch', async event => { a { color: #2d5589 } + #errors { + font-weight: bold; + font-size: smaller; + marin-top: 1em; + color: maroon; + opacity: 0.5; + font-family: monospace; + }

Still loading•••

attempts: 1

The content is still being loaded, thank you for your patience.

This page will auto-reload in a few seconds. If it does not, please click here.

+

` + + // return what we got out of that + return html +} + self.addEventListener('fetch', async event => { return void event.respondWith(async function () { // initialize the SW; this is necessary as SW can be stopped at any time @@ -1434,99 +1589,12 @@ self.addEventListener('fetch', async event => { //responseInit.headers['X-LibResilient-ETag'] = ??? responseInit.headers['X-LibResilient-Method'] = "still-loading" - // TODO: make this configurable via config.json - // TODO: https://gitlab.com/rysiekpl/libresilient/-/issues/82 - let stillLoadingHTML = `Still loading... - -

Still loading•••

-

attempts: 1

-

The content is still being loaded, thank you for your patience.

This page will auto-reload in a few seconds. If it does not, please click here.

-

- ` + // get the still-loading page contents + let stillLoadingHTML = getUserFacingHTML( + still_loading_messages, + success_messages, + failure_messages + ) let blob = new Blob( [stillLoadingHTML], From e443e26aaeca7d7cb1cd0016c82d3bda21ec9887 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20=27rysiek=27=20Wo=C5=BAniak?= Date: Sat, 2 Mar 2024 19:15:23 +0000 Subject: [PATCH 11/17] now displaying a user-facing HTML error page when a navigation request fails (ref. #36) --- service-worker.js | 193 ++++++++++++++++++++++++++++------------------ 1 file changed, 120 insertions(+), 73 deletions(-) diff --git a/service-worker.js b/service-worker.js index ffba823..2285ed9 100644 --- a/service-worker.js +++ b/service-worker.js @@ -1317,15 +1317,15 @@ self.addEventListener('activate', async event => { */ let still_loading_messages = { title: "Still loading", - body: "The content is still being loaded, thank you for your patience.

This page will auto-reload in a few seconds. If it does not, please click here." + body: "The resource is still being loaded, thank you for your patience.

This page will auto-reload in a few seconds. If it does not, please click here." } let success_messages = { title: "Loaded, redirecting!", - body: "The content has loaded, you are being redirected." + body: "The resource has loaded, you are being redirected." } let failure_messages = { title: "Loading failed.", - body: "We're sorry, we were unable to load this page." + body: "We're sorry, we were unable to load this resource." } /** @@ -1395,7 +1395,13 @@ let getUserFacingHTML = (init_msgs, success_msgs=false, failure_msgs=false) => { font-family: monospace; } -

${init_msgs.title}•••

+

${init_msgs.title}` + + if (success_msgs !== false) { + html += `•••` + } + + html += `

attempts: 1

${init_msgs.body}

@@ -1466,6 +1472,7 @@ let getUserFacingHTML = (init_msgs, success_msgs=false, failure_msgs=false) => { self.addEventListener('fetch', async event => { return void event.respondWith(async function () { + // initialize the SW; this is necessary as SW can be stopped at any time // and restarted when an event gets triggered -- `fetch` is just such an event. // @@ -1475,6 +1482,7 @@ self.addEventListener('fetch', async event => { // // the good news is that the config.json should have been cached already await initServiceWorker() + // if event.resultingClientId is available, we need to use this // otherwise event.clientId is what we want // ref. https://developer.mozilla.org/en-US/docs/Web/API/FetchEvent/resultingClientId @@ -1541,82 +1549,121 @@ self.addEventListener('fetch', async event => { // get handled by plugins in case of an error let lrPromise = getResourceThroughLibResilient(url, init, clientId) - // is the stillLoadingScreen enabled, and are we navigating, or just fetching some resource? - if ( ( self.LibResilientConfig.stillLoadingTimeout > 0 ) && ( event.request.mode === 'navigate' ) ) { + // are we navigating, or just fetching some resource? + if ( event.request.mode === 'navigate' ) { - self.log('service-worker', `handling a navigate request; still-loading timeout: ${self.LibResilientConfig.stillLoadingTimeout}.`) + // this is the promise we will want to return in the end + let finalPromise = null - let slPromise, slTimeoutId - [slPromise, slTimeoutId] = promiseTimeout(self.LibResilientConfig.stillLoadingTimeout, true) + // navigating! is the still-loading screen enabled? + if ( self.LibResilientConfig.stillLoadingTimeout > 0 ) { - // make sure to clear the timeout related to slPromise - // in case we manage to get the content through the plugins - lrPromise - .then(()=>{ - self.log('service-worker', `content retrieved; still-loading timeout cleared.`) - clearTimeout(slTimeoutId) - }) + // it is enabled! + self.log('service-worker', `handling a navigate request; still-loading timeout: ${self.LibResilientConfig.stillLoadingTimeout}.`) + + let slPromise, slTimeoutId + [slPromise, slTimeoutId] = promiseTimeout(self.LibResilientConfig.stillLoadingTimeout, true) + + // make sure to clear the timeout related to slPromise + // in case we manage to get the content through the plugins + lrPromise + .then(()=>{ + self.log('service-worker', `content retrieved; still-loading timeout cleared.`) + clearTimeout(slTimeoutId) + }) + + // prepare a Promise that races the "still loading" screen promise against the LibResilient plugins + finalPromise = Promise.race([ + // regular fetch-through-plugins + lrPromise, + + // the "still loading screen" + // + // this will delay a specified time, and ten return a Response + // with very basic HTML informing the user that the page is still loading, + // a Refresh header set, and a link for the user to reload the screen manually + slPromise + .then(()=>{ + + // inform + self.log('service-worker', 'handling a navigate request is taking too long, showing the still-loading screen') + + // we need to create a new Response object + // with all the headers added explicitly, + // since response.headers is immutable + var responseInit = { + status: 202, + statusText: "Accepted", + headers: {}, + url: url + }; + responseInit.headers['Content-Type'] = "text/html" + responseInit.headers['X-LibResilient-Method'] = "still-loading" + + // get the still-loading page contents + let stillLoadingHTML = getUserFacingHTML( + still_loading_messages, + success_messages, + failure_messages + ) + + let blob = new Blob( + [stillLoadingHTML], + {type: "text/html"} + ) + + return new Response( + blob, + responseInit + ) + }) + ]) + + // okay, no still-loading screen, but this is still a navigate request, so we want to display something to the user + } else { + self.log('service-worker', `handling a navigate request, but still-loading screen is disabled.`) + finalPromise = lrPromise + } - // return a Promise that races the "still loading" screen promise against the LibResilient plugins - return Promise.race([ - // regular fetch-through-plugins - lrPromise, - - // the "still loading screen" - // - // this will delay a specified time, and ten return a Response - // with very basic HTML informing the user that the page is still loading, - // a Refresh header set, and a link for the user to reload the screen manually - slPromise - .then(()=>{ - - // inform - self.log('service-worker', 'handling a navigate request is taking too long, showing the still-loading screen') - - // we need to create a new Response object - // with all the headers added explicitly, - // since response.headers is immutable - var responseInit = { - status: 202, - statusText: "Accepted", - headers: {}, - url: url - }; - responseInit.headers['Content-Type'] = "text/html" - // refresh: we want a minimum of 1s; stillLoadingTimeout is in ms! - //responseInit.headers['Refresh'] = Math.ceil( self.LibResilientConfig.stillLoadingTimeout / 1000 ) - //responseInit.headers['ETag'] = ??? - //responseInit.headers['X-LibResilient-ETag'] = ??? - responseInit.headers['X-LibResilient-Method'] = "still-loading" - - // get the still-loading page contents - let stillLoadingHTML = getUserFacingHTML( - still_loading_messages, - success_messages, - failure_messages - ) - - let blob = new Blob( - [stillLoadingHTML], - {type: "text/html"} - ) - - return new Response( - blob, - responseInit - ) - }) - ]) + // return finalPromise, with a catch! + return finalPromise.catch((e)=>{ + // inform + self.log('service-worker', 'handling a failed navigate request, showing the user-facing error screen') + + // we need to create a new Response object + // with all the headers added explicitly, + // since response.headers is immutable + var responseInit = { + status: 404, + statusText: "Not Found", + headers: {}, + url: url + }; + responseInit.headers['Content-Type'] = "text/html" + responseInit.headers['X-LibResilient-Method'] = "failed" + + // get the still-loading page contents + // it only needs to display the failure messages + let stillLoadingHTML = getUserFacingHTML( + failure_messages + ) + + let blob = new Blob( + [stillLoadingHTML], + {type: "text/html"} + ) + + return new Response( + blob, + responseInit + ) + }) // nope, just fetching a resource } else { - if ( event.request.mode === 'navigate' ) { - self.log('service-worker', `handling a navigate request, but still-loading screen is disabled.`) - } else { - self.log('service-worker', 'handling a regular request; still-loading screen will not be used.') - } - // no need for the whole "still loading screen" flow - return lrPromise; + // no need for a still-loading screen, no need for an user-facing error screen if the request fails + self.log('service-worker', 'handling a regular request; still-loading screen will not be used.') + return lrPromise } }()) }); 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 12/17] 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) }) } From b7419d0fe80410712c551a58a648843755e3e459 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 16:33:22 +0000 Subject: [PATCH 13/17] tests passing for work done so far for error handling improvements (ref. #36) --- plugins/any-of/__tests__/browser.test.js | 28 ---------- plugins/delay/__tests__/browser.test.js | 24 --------- .../dnslink-fetch/__tests__/browser.test.js | 54 ------------------- plugins/dnslink-fetch/index.js | 6 +-- plugins/fetch/__tests__/browser.test.js | 24 --------- plugins/test-plugin/__tests__/browser.test.js | 24 --------- plugins/test-plugin/index.js | 11 ++-- 7 files changed, 6 insertions(+), 165 deletions(-) diff --git a/plugins/any-of/__tests__/browser.test.js b/plugins/any-of/__tests__/browser.test.js index 1a9bb8d..af0665a 100644 --- a/plugins/any-of/__tests__/browser.test.js +++ b/plugins/any-of/__tests__/browser.test.js @@ -122,32 +122,4 @@ describe('browser: any-of plugin', async () => { }) assertEquals(await response.json(), {test: "success"}) }); - - it("should throw an error when HTTP status is >= 400", async () => { - - window.fetch = spy((url, init) => { - return Promise.resolve( - new Response( - new Blob( - ["Not Found"], - {type: "text/plain"} - ), - { - status: 404, - statusText: "Not Found" - } - ) - ) - }); - - assertRejects( - async () => { - return await LibResilientPluginConstructors - .get('any-of')(LR, init) - .fetch('https://resilient.is/test.json') }, - AggregateError, - 'All promises were rejected' - ) - assertSpyCalls(fetch, 1); - }); }) diff --git a/plugins/delay/__tests__/browser.test.js b/plugins/delay/__tests__/browser.test.js index 612290f..df92c55 100644 --- a/plugins/delay/__tests__/browser.test.js +++ b/plugins/delay/__tests__/browser.test.js @@ -99,28 +99,4 @@ describe('browser: fetch plugin', async () => { assertEquals(response.headers.get('X-LibResilient-ETag'), 'TestingETagHeader') }); - it("should throw an error when HTTP status is >= 400", async () => { - window.fetch = (url, init) => { - const response = new Response( - new Blob( - ["Not Found"], - {type: "text/plain"} - ), - { - status: 404, - statusText: "Not Found", - url: url - }); - return Promise.resolve(response); - } - assertRejects( - async () => { - return await LibResilientPluginConstructors - .get('fetch')(LR) - .fetch('https://resilient.is/test.json') }, - Error, - 'HTTP Error: 404 Not Found' - ) - }); - }) diff --git a/plugins/dnslink-fetch/__tests__/browser.test.js b/plugins/dnslink-fetch/__tests__/browser.test.js index ae7b26e..5fad586 100644 --- a/plugins/dnslink-fetch/__tests__/browser.test.js +++ b/plugins/dnslink-fetch/__tests__/browser.test.js @@ -1950,58 +1950,4 @@ describe('browser: dnslink-fetch plugin', async () => { assertEquals(response.headers.get('X-LibResilient-Method'), 'dnslink-fetch') assertEquals(response.headers.get('X-LibResilient-Etag'), 'TestingLastModifiedHeader') }); - - it("should throw an error when HTTP status is >= 400", async () => { - - window.resolvingFetch = (url, init) => { - if (url.startsWith('https://dns.hostux.net/dns-query')) { - const response = new Response( - new Blob( - [JSON.stringify(fetchResponse[0])], - {type: fetchResponse[1]} - ), - { - status: 200, - statusText: "OK", - headers: { - 'Last-Modified': 'TestingLastModifiedHeader' - }, - url: url - }); - return Promise.resolve(response); - } else { - const response = new Response( - new Blob( - ["Not Found"], - {type: "text/plain"} - ), - { - status: 404, - statusText: "Not Found", - url: url - }); - return Promise.resolve(response); - } - } - window.fetch = spy(window.resolvingFetch) - - window.fetchResponse = [ - {Status: 0, Answer: [ - {type: 16, data: 'dnslink=/https/example.org'}, - {type: 16, data: 'dnslink=/http/example.net/some/path'} - ]}, - "application/json" - ] - - assertRejects( - async ()=>{ - const response = await LibResilientPluginConstructors - .get('dnslink-fetch')(LR, init) - .fetch('https://resilient.is/test.json') - console.log(response) - }, - Error, - 'HTTP Error:' - ) - }); }) diff --git a/plugins/dnslink-fetch/index.js b/plugins/dnslink-fetch/index.js index 80e43c0..53d7846 100644 --- a/plugins/dnslink-fetch/index.js +++ b/plugins/dnslink-fetch/index.js @@ -226,11 +226,7 @@ u=>fetch(u, init) )) .then((response) => { - // 4xx? 5xx? that's a paddlin' - if (response.status >= 400) { - // throw an Error to fall back to other plugins: - throw new Error('HTTP Error: ' + response.status + ' ' + response.statusText); - } + // all good, it seems LR.log(pluginName, "fetched:", response.url); diff --git a/plugins/fetch/__tests__/browser.test.js b/plugins/fetch/__tests__/browser.test.js index 612290f..df92c55 100644 --- a/plugins/fetch/__tests__/browser.test.js +++ b/plugins/fetch/__tests__/browser.test.js @@ -99,28 +99,4 @@ describe('browser: fetch plugin', async () => { assertEquals(response.headers.get('X-LibResilient-ETag'), 'TestingETagHeader') }); - it("should throw an error when HTTP status is >= 400", async () => { - window.fetch = (url, init) => { - const response = new Response( - new Blob( - ["Not Found"], - {type: "text/plain"} - ), - { - status: 404, - statusText: "Not Found", - url: url - }); - return Promise.resolve(response); - } - assertRejects( - async () => { - return await LibResilientPluginConstructors - .get('fetch')(LR) - .fetch('https://resilient.is/test.json') }, - Error, - 'HTTP Error: 404 Not Found' - ) - }); - }) diff --git a/plugins/test-plugin/__tests__/browser.test.js b/plugins/test-plugin/__tests__/browser.test.js index 612290f..df92c55 100644 --- a/plugins/test-plugin/__tests__/browser.test.js +++ b/plugins/test-plugin/__tests__/browser.test.js @@ -99,28 +99,4 @@ describe('browser: fetch plugin', async () => { assertEquals(response.headers.get('X-LibResilient-ETag'), 'TestingETagHeader') }); - it("should throw an error when HTTP status is >= 400", async () => { - window.fetch = (url, init) => { - const response = new Response( - new Blob( - ["Not Found"], - {type: "text/plain"} - ), - { - status: 404, - statusText: "Not Found", - url: url - }); - return Promise.resolve(response); - } - assertRejects( - async () => { - return await LibResilientPluginConstructors - .get('fetch')(LR) - .fetch('https://resilient.is/test.json') }, - Error, - 'HTTP Error: 404 Not Found' - ) - }); - }) diff --git a/plugins/test-plugin/index.js b/plugins/test-plugin/index.js index 521c0e5..fd82648 100644 --- a/plugins/test-plugin/index.js +++ b/plugins/test-plugin/index.js @@ -32,12 +32,11 @@ // run built-in regular fetch() return fetch(url, init) .then(async (response) => { - // 4xx? 5xx? that's a paddlin' - if (response.status >= 400) { - // throw an Error to fall back to LibResilient: - throw new Error('HTTP Error: ' + response.status + ' ' + response.statusText); - } - // all good, it seems + + // we got something, it seems + // it might be a 2xx; it might be a 3xx redirect + // it might also be a 4xx or a 5xx error + // the service worker will know how to deal with those LR.log(pluginName, `fetched successfully: ${response.url}`); // we need to create a new Response object From e4db403d62d6751c8e6216df546fc63ee5f88525 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20=27rysiek=27=20Wo=C5=BAniak?= Date: Wed, 13 Mar 2024 02:48:29 +0000 Subject: [PATCH 14/17] additional tests for handling errors/rejections in plugins (ref. #36) --- .../service-worker/service-worker.test.js | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/__tests__/service-worker/service-worker.test.js b/__tests__/service-worker/service-worker.test.js index 5a1d814..8aa341a 100644 --- a/__tests__/service-worker/service-worker.test.js +++ b/__tests__/service-worker/service-worker.test.js @@ -2882,4 +2882,110 @@ describe('service-worker', async () => { assertEquals(await response.json(), { test: "success" }) }) + + it("should return a 404 Not Found HTTP response object when handling a failed navigation request", async () => { + window.LibResilientConfig = { + plugins: [{ + name: 'reject-all' + }], + loggedComponents: [ + 'service-worker' + ] + } + + let rejectingFetch = spy( + (request, init)=>{ return Promise.reject('reject-all rejecting a request for: ' + request); } + ) + + window.LibResilientPluginConstructors.set('reject-all', ()=>{ + return { + name: 'reject-all', + description: 'Reject all requests.', + version: '0.0.1', + fetch: rejectingFetch + } + }) + + await import("../../service-worker.js?" + window.test_id); + await self.dispatchEvent(new Event('install')) + await self.waitForSWInstall() + + let fetch_event = new FetchEvent(window.location.origin + 'test.json', {mode: "navigate"}) + window.dispatchEvent(fetch_event) + let response = await fetch_event.waitForResponse() + + assertEquals(response.status, 404) + assertEquals(response.statusText, 'Not Found') + assertEquals(response.headers.get('content-type'), 'text/html') + assertEquals((await response.text()).slice(0, 57), 'Loading failed.') + }) + + it("should not return a 404 Not Found HTTP response object when handling a rejected non-navigation request", async () => { + window.LibResilientConfig = { + plugins: [{ + name: 'reject-all' + }], + loggedComponents: [ + 'service-worker' + ] + } + + let rejectingFetch = spy( + (request, init)=>{ return Promise.reject('reject-all rejecting a request for: ' + request); } + ) + + window.LibResilientPluginConstructors.set('reject-all', ()=>{ + return { + name: 'reject-all', + description: 'Reject all requests.', + version: '0.0.1', + fetch: rejectingFetch + } + }) + + await import("../../service-worker.js?" + window.test_id); + await self.dispatchEvent(new Event('install')) + await self.waitForSWInstall() + + let fetch_event = new FetchEvent(window.location.origin + 'test.json') + window.dispatchEvent(fetch_event) + assertRejects(async ()=>{ await fetch_event.waitForResponse() }) + }) + + it("should not return a 404 Not Found HTTP response object when handling a non-navigation request that throws an error", async () => { + window.LibResilientConfig = { + plugins: [{ + name: 'error-out' + }], + loggedComponents: [ + 'service-worker' + ] + } + + let throwingFetch = spy( + (request, init)=>{ throw new Error('error-out throwing an Error for: ' + request); } + ) + + window.LibResilientPluginConstructors.set('error-out', ()=>{ + return { + name: 'error-out', + description: 'Throws.', + version: '0.0.1', + fetch: throwingFetch + } + }) + + await import("../../service-worker.js?" + window.test_id); + await self.dispatchEvent(new Event('install')) + await self.waitForSWInstall() + + let fetch_event = new FetchEvent(window.location.origin + 'test.json') + window.dispatchEvent(fetch_event) + assertRejects(async ()=>{ + await fetch_event.waitForResponse() + }, + Error, + 'error-out throwing an Error for: https://test.resilient.is/test.json' + ) + }) }) From 935b9c27aceb289d3585480fabca963855ac490f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20=27rysiek=27=20Wo=C5=BAniak?= Date: Wed, 13 Mar 2024 03:20:10 +0000 Subject: [PATCH 15/17] additional tests for handling HTTP errors from plugins (ref. #36) --- .../service-worker/service-worker.test.js | 246 +++++++++++++++++- 1 file changed, 243 insertions(+), 3 deletions(-) diff --git a/__tests__/service-worker/service-worker.test.js b/__tests__/service-worker/service-worker.test.js index 8aa341a..43c9601 100644 --- a/__tests__/service-worker/service-worker.test.js +++ b/__tests__/service-worker/service-worker.test.js @@ -1592,6 +1592,246 @@ describe('service-worker', async () => { assertEquals(await response.json(), { test: "success" }) }); + it("should use return a 4xx error directly from the last plugin, regardless of previous plugin errors or rejection", async () => { + window.LibResilientConfig = { + plugins: [{ + name: 'reject-all' + },{ + name: 'error-out' + },{ + name: 'return-418' + }], + loggedComponents: [ + 'service-worker' + ] + } + + let rejectingFetch = spy( + (request, init)=>{ return Promise.reject('reject-all rejecting a request for: ' + request); } + ) + + window.LibResilientPluginConstructors.set('reject-all', ()=>{ + return { + name: 'reject-all', + description: 'Reject all requests.', + version: '0.0.1', + fetch: rejectingFetch + } + }) + + let throwingFetch = spy( + (request, init)=>{ throw new Error('error-out throwing an Error for: ' + request); } + ) + + window.LibResilientPluginConstructors.set('error-out', ()=>{ + return { + name: 'error-out', + description: 'Throws.', + version: '0.0.1', + fetch: throwingFetch + } + }) + + let mock_response_data = { + data: JSON.stringify({text: "success"}), + status: 418, + statusText: "Im A Teapot" + } + window.fetch = spy(window.getMockedFetch(mock_response_data)) + + + window.LibResilientPluginConstructors.set('return-418', ()=>{ + return { + name: 'return-418', + description: 'Return 418 HTTP Error.', + version: '0.0.1', + fetch: fetch + } + }) + + await import("../../service-worker.js?" + window.test_id); + await self.dispatchEvent(new Event('install')) + await self.waitForSWInstall() + + let fetch_event = new FetchEvent('test.json') + window.dispatchEvent(fetch_event) + let response = await fetch_event.waitForResponse() + + assertSpyCalls(window.fetch, 2); // two, because the first one is for config.json + assertSpyCalls(rejectingFetch, 1); + assertSpyCalls(throwingFetch, 1); + assertSpyCall(window.fetch, 1, { args: [ + "https://test.resilient.is/test.json", + { + cache: undefined, + integrity: undefined, + method: "GET", + redirect: "follow", + referrer: undefined, + }] + }) + assertEquals(response.status, 418) + assertEquals(response.statusText, 'Im A Teapot') + assertEquals(await response.json(), { text: "success" }) + }); + + it("should use return a 4xx error directly from a plugin, regardless of any following plugins", async () => { + window.LibResilientConfig = { + plugins: [{ + name: 'return-418' + },{ + name: 'reject-all' + },{ + name: 'error-out' + }], + loggedComponents: [ + 'service-worker' + ] + } + + let rejectingFetch = spy( + (request, init)=>{ return Promise.reject('reject-all rejecting a request for: ' + request); } + ) + + window.LibResilientPluginConstructors.set('reject-all', ()=>{ + return { + name: 'reject-all', + description: 'Reject all requests.', + version: '0.0.1', + fetch: rejectingFetch + } + }) + + let throwingFetch = spy( + (request, init)=>{ throw new Error('error-out throwing an Error for: ' + request); } + ) + + window.LibResilientPluginConstructors.set('error-out', ()=>{ + return { + name: 'error-out', + description: 'Throws.', + version: '0.0.1', + fetch: throwingFetch + } + }) + + let mock_response_data = { + data: JSON.stringify({text: "success"}), + status: 418, + statusText: "Im A Teapot" + } + window.fetch = spy(window.getMockedFetch(mock_response_data)) + + + window.LibResilientPluginConstructors.set('return-418', ()=>{ + return { + name: 'return-418', + description: 'Return 418 HTTP Error.', + version: '0.0.1', + fetch: fetch + } + }) + + await import("../../service-worker.js?" + window.test_id); + await self.dispatchEvent(new Event('install')) + await self.waitForSWInstall() + + let fetch_event = new FetchEvent('test.json') + window.dispatchEvent(fetch_event) + let response = await fetch_event.waitForResponse() + + assertSpyCalls(window.fetch, 2); // two, because the first one is for config.json + assertSpyCalls(rejectingFetch, 0); + assertSpyCalls(throwingFetch, 0); + assertSpyCall(window.fetch, 1, { args: [ + "https://test.resilient.is/test.json", + { + cache: undefined, + integrity: undefined, + method: "GET", + redirect: "follow", + referrer: undefined, + }] + }) + assertEquals(response.status, 418) + assertEquals(response.statusText, 'Im A Teapot') + assertEquals(await response.json(), { text: "success" }) + }); + + it("should use treat a 5xx error from a plugin as internal error and try following plugins", async () => { + window.LibResilientConfig = { + plugins: [{ + name: 'return-500' + },{ + name: 'reject-all' + },{ + name: 'error-out' + }], + loggedComponents: [ + 'service-worker' + ] + } + + let rejectingFetch = spy( + (request, init)=>{ return Promise.reject('reject-all rejecting a request for: ' + request); } + ) + + window.LibResilientPluginConstructors.set('reject-all', ()=>{ + return { + name: 'reject-all', + description: 'Reject all requests.', + version: '0.0.1', + fetch: rejectingFetch + } + }) + + let throwingFetch = spy( + (request, init)=>{ throw new Error('error-out throwing an Error for: ' + request); } + ) + + window.LibResilientPluginConstructors.set('error-out', ()=>{ + return { + name: 'error-out', + description: 'Throws.', + version: '0.0.1', + fetch: throwingFetch + } + }) + + let mock_response_data = { + data: JSON.stringify({text: "success"}), + status: 500, + statusText: "Internal Server Error" + } + window.fetch = spy(window.getMockedFetch(mock_response_data)) + + window.LibResilientPluginConstructors.set('return-500', ()=>{ + return { + name: 'return-500', + description: 'Return 500 HTTP Error.', + version: '0.0.1', + fetch: fetch + } + }) + + await import("../../service-worker.js?" + window.test_id); + await self.dispatchEvent(new Event('install')) + await self.waitForSWInstall() + + let fetch_event = new FetchEvent('test.json') + window.dispatchEvent(fetch_event) + let response = fetch_event.waitForResponse() + assertRejects( async () => { + return await response + }) + // wait for the response to resolve + await response.catch((e)=>{}) + + assertSpyCalls(window.fetch, 2); + assertSpyCalls(rejectingFetch, 1); + assertSpyCalls(throwingFetch, 1); + }); + it("should normalize query params in requested URLs by default", async () => { console.log(self.LibResilientConfig) @@ -2883,7 +3123,7 @@ describe('service-worker', async () => { assertEquals(await response.json(), { test: "success" }) }) - it("should return a 404 Not Found HTTP response object when handling a failed navigation request", async () => { + it("should return a 404 Not Found HTTP response object with an error screen when handling a rejected navigation request", async () => { window.LibResilientConfig = { plugins: [{ name: 'reject-all' @@ -2920,7 +3160,7 @@ describe('service-worker', async () => { assertEquals((await response.text()).slice(0, 57), 'Loading failed.') }) - it("should not return a 404 Not Found HTTP response object when handling a rejected non-navigation request", async () => { + it("should not return a 404 Not Found HTTP response object with an error screen when handling a rejected non-navigation request", async () => { window.LibResilientConfig = { plugins: [{ name: 'reject-all' @@ -2952,7 +3192,7 @@ describe('service-worker', async () => { assertRejects(async ()=>{ await fetch_event.waitForResponse() }) }) - it("should not return a 404 Not Found HTTP response object when handling a non-navigation request that throws an error", async () => { + it("should not return a 404 Not Found HTTP response object with an error screen when handling a non-navigation request that throws an error", async () => { window.LibResilientConfig = { plugins: [{ name: 'error-out' From c57381076280a083ebffdae4bc2cc8d64a58fa71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20=27rysiek=27=20Wo=C5=BAniak?= Date: Wed, 13 Mar 2024 03:35:19 +0000 Subject: [PATCH 16/17] additional test for activate event handling --- __tests__/service-worker/service-worker.test.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/__tests__/service-worker/service-worker.test.js b/__tests__/service-worker/service-worker.test.js index 43c9601..9aa522a 100644 --- a/__tests__/service-worker/service-worker.test.js +++ b/__tests__/service-worker/service-worker.test.js @@ -232,9 +232,9 @@ beforeAll(async ()=>{ postMessage: window.clients.prototypePostMessage } }, - claim: async () => { + claim: spy(async () => { return undefined - }, + }), // the actual spy function must be possible to reference // but we want spy data per test, so we set it properly in beforeEach() prototypePostMessage: null @@ -443,6 +443,14 @@ describe('service-worker', async () => { window.test_id = 0 + it("should call clients.claim() when activated", async () => { + await import("../../service-worker.js?" + window.test_id); + await self.dispatchEvent(new Event('install')) + await self.waitForSWInstall() + await self.dispatchEvent(new Event('activate')) + assertSpyCalls(window.clients.claim, 1) + }) + it("should use default LibResilientConfig values when config.json is missing", async () => { let mock_response_data = { From 562a62df11375529261caf96e9609cf1e3d531fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20=27rysiek=27=20Wo=C5=BAniak?= Date: Wed, 13 Mar 2024 04:07:29 +0000 Subject: [PATCH 17/17] documentation updated (ref. #36) --- docs/ARCHITECTURE.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 253de8f..e43ebbe 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -219,3 +219,31 @@ The still-loading screen is *only* displayed when *all* of these conditions are The reason why a stashing plugin needs to be configured and enabled is to avoid loops. Consider a scenario, where a visitor is navigating to a page, and the request is taking very long. The still-loading screen is displayed (by way of the service worker returning the relevant HTML in response to the request). Eventually, the request completes in the background, but the response is discarded due to lack of a stashing plugin. In such a case reloading the page will cause a repeat: request, still-loading screen, request completes in the background (and the result is discarded). The visitor would be stuck in a loop. If a stashing plugin (like `cache`) is enabled, this loop can be expected not to emerge, since the second request would quickly return the cached response. + +## Error handling + +LibResilient's error handling focuses on attempting to "do the right thing". In some cases this means passing a HTTP error response from a plugin directly to the browser to be displayed to the user; in other cases it means ignoring HTTP error response from a plugin so that other plugins can attempt to retrieve a given resource. + +In general: + + - **A response with status code value of `499` or lower is passed immediately to the browser** + no other plugins are used to try to retrieve the content; if a stashing plugin is configured, the response might be cached locally. + + - **A response with status code value of `500` or higher is treated as a plugin error** + if other plugins are configured, they will be used to try to retrieve the content; if a stashing plugin is configured it will not stash that response. + + - **Any exception thrown in a plugin will be caught and treated as a plugin error** + if other plugins are configured, they will be used to try to retrieve the content; there is no response object, so there is nothing to stash. + + - **If a plugin rejects for whatever reason, it is treated as a plugin error** + if other plugins are configured, they will be used to try to retrieve the content; there is no response object, so there is nothing to stash. + +All plugin errors (`5xx` HTTP responses, thrown exceptions, rejections) are logged internally. This data is printed in the console (if `loggedComponents` config field contains `service-worker`), and sent to the client using `Client.postMessage()`, to simplify debugging. + +If all plugins fail in case of a navigate request, a Request object is created with a `404 Not Found` HTTP status, containing a simple HTML error page (similar to the still-loading screen mentioned above) to be displayed to the user. If the request is not a navigate request, the rejected promise is returned directly. + +Mapping plugin errors onto HTTP errors is not always going to be trivial. For example, an IPFS-based transport plugin could in some circumstances return a `404 Not Found` HTTP error, but the `any-of` plugin necessarily has to ignore any HTTP errors it receives from plugins it is configured to use, while waiting for one to potentially return the resource successfully. If all of the configured plugins fail, with different HTTP errors, which one should the `any-of` plugin return itself?.. + +At the same time, returning HTTP errors makes sense, as it allows the browser and the user to properly interpret well-understood errors. So, the `fetch` plugin will return any `4xx` HTTP error it receives, for example, and the service worker will in turn treat that as a successfully completed retrieval and return that to the browser to be displayed to the user. + +Plugin authors should consider this carefully. If in doubt, it's probably better to throw an exception or reject the promise with a meaningful error message, than to try to fit a potentially complex failure mode into the limited and rigit contraints of HTTP error codes.