From 4e4fa49b0b1fd6ec5a018e742eb9910aa32e7637 Mon Sep 17 00:00:00 2001 From: toasted-nutbread Date: Sat, 28 May 2022 21:55:37 -0400 Subject: [PATCH] Audio request errors (#2161) * Generalize _onBeforeSendHeadersAddListener * Simplify filter assignment * Use requestId rather than done * Properly support Firefox addListener without arguments * Add details to fetchAnonymous errors * Refactor * Enable support for no header modifications * Update MV3 support for error details * Expose errors in downloadTermAudio * Throw an error if audio download fails due to potential permissions reasons --- dev/data/manifest-variants.json | 1 - ext/js/background/backend.js | 24 +++++ ext/js/background/request-builder.js | 128 +++++++++++++++++---------- ext/js/media/audio-downloader.js | 7 +- 4 files changed, 109 insertions(+), 51 deletions(-) diff --git a/dev/data/manifest-variants.json b/dev/data/manifest-variants.json index 419a03e3..96b6d877 100644 --- a/dev/data/manifest-variants.json +++ b/dev/data/manifest-variants.json @@ -172,7 +172,6 @@ {"action": "move", "path": ["content_security_policy_old"], "newPath": ["content_security_policy", "extension_pages"]}, {"action": "move", "path": ["sandbox", "content_security_policy"], "newPath": ["content_security_policy", "sandbox"]}, {"action": "remove", "path": ["permissions"], "item": ""}, - {"action": "remove", "path": ["permissions"], "item": "webRequest"}, {"action": "remove", "path": ["permissions"], "item": "webRequestBlocking"}, {"action": "add", "path": ["permissions"], "items": ["declarativeNetRequest", "scripting"]}, {"action": "set", "path": ["host_permissions"], "value": [""], "after": "optional_permissions"}, diff --git a/ext/js/background/backend.js b/ext/js/background/backend.js index b25b6033..c0f286f8 100644 --- a/ext/js/background/backend.js +++ b/ext/js/background/backend.js @@ -1803,6 +1803,8 @@ class Backend { reading )); } catch (e) { + const error = this._getAudioDownloadError(e); + if (error !== null) { throw error; } // No audio return null; } @@ -1894,6 +1896,28 @@ class Backend { return {results, errors}; } + _getAudioDownloadError(error) { + if (isObject(error.data)) { + const {errors} = error.data; + if (Array.isArray(errors)) { + for (const error2 of errors) { + if (!isObject(error2.data)) { continue; } + const {details} = error2.data; + if (!isObject(details)) { continue; } + if (details.error === 'net::ERR_FAILED') { + // This is potentially an error due to the extension not having enough URL privileges. + // The message logged to the console looks like this: + // Access to fetch at '' from origin 'chrome-extension://' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource. If an opaque response serves your needs, set the request's mode to 'no-cors' to fetch the resource with CORS disabled. + const result = new Error('Audio download failed due to possible extension permissions error'); + result.data = {errors}; + return result; + } + } + } + } + return null; + } + _generateAnkiNoteMediaFileName(prefix, extension, timestamp, definitionDetails) { let fileName = prefix; diff --git a/ext/js/background/request-builder.js b/ext/js/background/request-builder.js index 6c99acd6..ad1536f1 100644 --- a/ext/js/background/request-builder.js +++ b/ext/js/background/request-builder.js @@ -17,7 +17,6 @@ class RequestBuilder { constructor() { - this._extraHeadersSupported = null; this._onBeforeSendHeadersExtraInfoSpec = ['blocking', 'requestHeaders', 'extraHeaders']; this._textEncoder = new TextEncoder(); this._ruleIds = new Set(); @@ -36,74 +35,107 @@ class RequestBuilder { return await this._fetchAnonymousDeclarative(url, init); } const originURL = this._getOriginURL(url); - const modifications = [ + const headerModifications = [ ['cookie', null], ['origin', {name: 'Origin', value: originURL}] ]; - return await this._fetchModifyHeaders(url, init, modifications); + return await this._fetchInternal(url, init, headerModifications); } // Private - async _fetchModifyHeaders(url, init, modifications) { - const matchURL = this._getMatchURL(url); - - let done = false; - const callback = (details) => { - if (done || details.url !== url) { return {}; } - done = true; - - const requestHeaders = details.requestHeaders; - this._modifyHeaders(requestHeaders, modifications); - return {requestHeaders}; - }; + async _fetchInternal(url, init, headerModifications) { const filter = { - urls: [matchURL], + urls: [this._getMatchURL(url)], types: ['xmlhttprequest'] }; - let needsCleanup = false; - try { - this._onBeforeSendHeadersAddListener(callback, filter); - needsCleanup = true; - } catch (e) { - // NOP - } + let requestId = null; + const onBeforeSendHeadersCallback = (details) => { + if (requestId !== null || details.url !== url) { return {}; } + ({requestId} = details); + + if (headerModifications === null) { return {}; } + + const requestHeaders = details.requestHeaders; + this._modifyHeaders(requestHeaders, headerModifications); + return {requestHeaders}; + }; + + let errorDetailsTimer = null; + let {promise: errorDetailsPromise, resolve: errorDetailsResolve} = deferPromise(); + const onErrorOccurredCallback = (details) => { + if (errorDetailsResolve === null || details.requestId !== requestId) { return; } + if (errorDetailsTimer !== null) { + clearTimeout(errorDetailsTimer); + errorDetailsTimer = null; + } + errorDetailsResolve(details); + errorDetailsResolve = null; + }; + + const eventListeners = []; + const onBeforeSendHeadersExtraInfoSpec = (headerModifications !== null ? this._onBeforeSendHeadersExtraInfoSpec : []); + this._addWebRequestEventListener(chrome.webRequest.onBeforeSendHeaders, onBeforeSendHeadersCallback, filter, onBeforeSendHeadersExtraInfoSpec, eventListeners); + this._addWebRequestEventListener(chrome.webRequest.onErrorOccurred, onErrorOccurredCallback, filter, void 0, eventListeners); try { return await fetch(url, init); - } finally { - if (needsCleanup) { - try { - chrome.webRequest.onBeforeSendHeaders.removeListener(callback); - } catch (e) { - // NOP - } + } catch (e) { + // onErrorOccurred is not always invoked by this point, so a delay is needed + if (errorDetailsResolve !== null) { + errorDetailsTimer = setTimeout(() => { + errorDetailsTimer = null; + if (errorDetailsResolve === null) { return; } + errorDetailsResolve(null); + errorDetailsResolve = null; + }, 100); } + const details = await errorDetailsPromise; + e.data = {details}; + throw e; + } finally { + this._removeWebRequestEventListeners(eventListeners); } } - _onBeforeSendHeadersAddListener(callback, filter) { - const extraInfoSpec = this._onBeforeSendHeadersExtraInfoSpec; - for (let i = 0; i < 2; ++i) { - try { - chrome.webRequest.onBeforeSendHeaders.addListener(callback, filter, extraInfoSpec); - if (this._extraHeadersSupported === null) { - this._extraHeadersSupported = true; - } - break; - } catch (e) { - // Firefox doesn't support the 'extraHeaders' option and will throw the following error: - // Type error for parameter extraInfoSpec (Error processing 2: Invalid enumeration value "extraHeaders") for webRequest.onBeforeSendHeaders. - if (this._extraHeadersSupported !== null || !`${e.message}`.includes('extraHeaders')) { + _addWebRequestEventListener(target, callback, filter, extraInfoSpec, eventListeners) { + try { + for (let i = 0; i < 2; ++i) { + try { + if (typeof extraInfoSpec === 'undefined') { + target.addListener(callback, filter); + } else { + target.addListener(callback, filter, extraInfoSpec); + } + break; + } catch (e) { + // Firefox doesn't support the 'extraHeaders' option and will throw the following error: + // Type error for parameter extraInfoSpec (Error processing 2: Invalid enumeration value "extraHeaders") for [target]. + if (i === 0 && `${e.message}`.includes('extraHeaders') && Array.isArray(extraInfoSpec)) { + const index = extraInfoSpec.indexOf('extraHeaders'); + if (index >= 0) { + extraInfoSpec.splice(index, 1); + continue; + } + } throw e; } } + } catch (e) { + console.log(e); + return; + } + eventListeners.push({target, callback}); + } - // addListener failed; remove 'extraHeaders' from extraInfoSpec. - this._extraHeadersSupported = false; - const index = extraInfoSpec.indexOf('extraHeaders'); - if (index >= 0) { extraInfoSpec.splice(index, 1); } + _removeWebRequestEventListeners(eventListeners) { + for (const {target, callback} of eventListeners) { + try { + target.removeListener(callback); + } catch (e) { + console.log(e); + } } } @@ -197,7 +229,7 @@ class RequestBuilder { await this._updateDynamicRules({addRules}); try { - return await fetch(url, init); + return await this._fetchInternal(url, init, null); } finally { await this._tryUpdateDynamicRules({removeRuleIds: [id]}); } diff --git a/ext/js/media/audio-downloader.js b/ext/js/media/audio-downloader.js index c2e1742f..0991d14d 100644 --- a/ext/js/media/audio-downloader.js +++ b/ext/js/media/audio-downloader.js @@ -51,6 +51,7 @@ class AudioDownloader { } async downloadTermAudio(sources, preferredAudioIndex, term, reading) { + const errors = []; for (const source of sources) { let infoList = await this.getTermAudioInfoList(source, term, reading); if (typeof preferredAudioIndex === 'number') { @@ -62,14 +63,16 @@ class AudioDownloader { try { return await this._downloadAudioFromUrl(info.url, source.type); } catch (e) { - // NOP + errors.push(e); } break; } } } - throw new Error('Could not download audio'); + const error = new Error('Could not download audio'); + error.data = {errors}; + throw error; } // Private