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

@@ -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": "<all_urls>"},
-                {"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": ["<all_urls>"], "after": "optional_permissions"},

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 '<URL>' from origin 'chrome-extension://<ID>' 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;
 

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) {
+    async _fetchInternal(url, init, headerModifications) {
-        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};
-        };
         const filter = {
-            urls: [matchURL],
+            urls: [this._getMatchURL(url)],
             types: ['xmlhttprequest']
         };
 
-        let needsCleanup = false;
+        let requestId = null;
-        try {
+        const onBeforeSendHeadersCallback = (details) => {
-            this._onBeforeSendHeadersAddListener(callback, filter);
+            if (requestId !== null || details.url !== url) { return {}; }
-            needsCleanup = true;
+            ({requestId} = details);
-        } catch (e) {
+
-            // NOP
+            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
+            // 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) {
+    _addWebRequestEventListener(target, callback, filter, extraInfoSpec, eventListeners) {
-        const extraInfoSpec = this._onBeforeSendHeadersExtraInfoSpec;
+        try {
             for (let i = 0; i < 2; ++i) {
                 try {
-                chrome.webRequest.onBeforeSendHeaders.addListener(callback, filter, extraInfoSpec);
+                    if (typeof extraInfoSpec === 'undefined') {
-                if (this._extraHeadersSupported === null) {
+                        target.addListener(callback, filter);
-                    this._extraHeadersSupported = true;
+                    } 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 webRequest.onBeforeSendHeaders.
+                    // Type error for parameter extraInfoSpec (Error processing 2: Invalid enumeration value "extraHeaders") for [target].
-                if (this._extraHeadersSupported !== null || !`${e.message}`.includes('extraHeaders')) {
+                    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.
+    _removeWebRequestEventListeners(eventListeners) {
-            this._extraHeadersSupported = false;
+        for (const {target, callback} of eventListeners) {
-            const index = extraInfoSpec.indexOf('extraHeaders');
+            try {
-            if (index >= 0) { extraInfoSpec.splice(index, 1); }
+                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]});
             }

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