From 310303ca1a123a77f9bd116af4dc64ad9c3256c5 Mon Sep 17 00:00:00 2001 From: toasted-nutbread Date: Sat, 20 Aug 2022 11:17:24 -0400 Subject: Audio download timeout (#2187) * Add support for an idle timeout when downloading audio * Update eslint rules * Pass idleTimeout to the downloader from DisplayAnki * Add anki.downloadTimeout setting * Update tests * Assign _audioDownloadIdleTimeout using settings * Show info about cancelled downloads * Handle Firefox bug * Improve audio errors * Refactor * Move functions to RequestBuilder --- .eslintrc.json | 8 ++++ ext/data/schemas/options-schema.json | 8 +++- ext/issues.html | 12 +++++ ext/js/background/backend.js | 32 +++++++++---- ext/js/background/request-builder.js | 90 +++++++++++++++++++++++++++++++++++- ext/js/data/anki-note-builder.js | 4 +- ext/js/data/options-util.js | 12 ++++- ext/js/display/display-anki.js | 24 +++++++++- ext/js/media/audio-downloader.js | 32 +++++++++++-- ext/settings.html | 26 ++++++++++- test/test-options-util.js | 5 +- 11 files changed, 230 insertions(+), 23 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index 02660d0a..dad145be 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -291,6 +291,14 @@ "rules": { "no-implicit-globals": "error" } + }, + { + "files": [ + "ext/js/**/*.js" + ], + "globals": { + "AbortController": "readonly" + } } ] } diff --git a/ext/data/schemas/options-schema.json b/ext/data/schemas/options-schema.json index d7e3b5f4..37b32bbf 100644 --- a/ext/data/schemas/options-schema.json +++ b/ext/data/schemas/options-schema.json @@ -878,7 +878,8 @@ "suspendNewCards", "displayTags", "noteGuiMode", - "apiKey" + "apiKey", + "downloadTimeout" ], "properties": { "enable": { @@ -1002,6 +1003,11 @@ "apiKey": { "type": "string", "default": "" + }, + "downloadTimeout": { + "type": "number", + "default": 0, + "minimum": 0 } } }, diff --git a/ext/issues.html b/ext/issues.html index 4d74ae76..df6e6135 100644 --- a/ext/issues.html +++ b/ext/issues.html @@ -42,6 +42,18 @@ +

Audio download was cancelled due to an idle timeout

+
+
+

+ Audio files can be downloaded from remote servers when creating Anki cards, + and sometimes these downloads can stall due to server or internet connectivity issues. + The Idle download timeout setting on the settings page + specifies a time limit for stalled downloads. +

+
+
+
diff --git a/ext/js/background/backend.js b/ext/js/background/backend.js index 75ff7bee..f3c76311 100644 --- a/ext/js/background/backend.js +++ b/ext/js/background/backend.js @@ -1809,7 +1809,7 @@ class Backend { return null; } - const {sources, preferredAudioIndex} = details; + const {sources, preferredAudioIndex, idleTimeout} = details; let data; let contentType; try { @@ -1817,7 +1817,8 @@ class Backend { sources, preferredAudioIndex, term, - reading + reading, + idleTimeout )); } catch (e) { const error = this._getAudioDownloadError(e); @@ -1918,6 +1919,9 @@ class Backend { const {errors} = error.data; if (Array.isArray(errors)) { for (const error2 of errors) { + if (error2.name === 'AbortError') { + return this._createAudioDownloadError('Audio download was cancelled due to an idle timeout', 'audio-download-idle-timeout', errors); + } if (!isObject(error2.data)) { continue; } const {details} = error2.data; if (!isObject(details)) { continue; } @@ -1925,12 +1929,7 @@ class Backend { // 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, - referenceUrl: '/issues.html#audio-download-failed' - }; - return result; + return this._createAudioDownloadError('Audio download failed due to possible extension permissions error', 'audio-download-failed', errors); } } } @@ -1938,6 +1937,23 @@ class Backend { return null; } + _createAudioDownloadError(message, issueId, errors) { + const error = new Error(message); + const hasErrors = Array.isArray(errors); + const hasIssueId = (typeof issueId === 'string'); + if (hasErrors || hasIssueId) { + error.data = {}; + if (hasErrors) { + // Errors need to be serialized since they are passed to other frames + error.data.errors = errors.map((e) => serializeError(e)); + } + if (hasIssueId) { + error.data.referenceUrl = `/issues.html#${issueId}`; + } + } + return error; + } + _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 ad1536f1..2cdd6f0e 100644 --- a/ext/js/background/request-builder.js +++ b/ext/js/background/request-builder.js @@ -42,6 +42,58 @@ class RequestBuilder { return await this._fetchInternal(url, init, headerModifications); } + static async readFetchResponseArrayBuffer(response, onProgress) { + let reader; + try { + if (typeof onProgress === 'function') { + reader = response.body.getReader(); + } + } catch (e) { + // Not supported + } + + if (typeof reader === 'undefined') { + const result = await response.arrayBuffer(); + if (typeof onProgress === 'function') { + onProgress(true); + } + return result; + } + + const contentLengthString = response.headers.get('Content-Length'); + const contentLength = contentLengthString !== null ? Number.parseInt(contentLengthString, 10) : null; + let target = Number.isFinite(contentLength) ? new Uint8Array(contentLength) : null; + let targetPosition = 0; + let totalLength = 0; + const targets = []; + + while (true) { + const {done, value} = await reader.read(); + if (done) { break; } + onProgress(false); + if (target === null) { + targets.push({array: value, length: value.length}); + } else if (targetPosition + value.length > target.length) { + targets.push({array: target, length: targetPosition}); + target = null; + } else { + target.set(value, targetPosition); + targetPosition += value.length; + } + totalLength += value.length; + } + + if (target === null) { + target = this._joinUint8Arrays(targets, totalLength); + } else if (totalLength < target.length) { + target = target.slice(0, totalLength); + } + + onProgress(true); + + return target; + } + // Private async _fetchInternal(url, init, headerModifications) { @@ -92,7 +144,10 @@ class RequestBuilder { }, 100); } const details = await errorDetailsPromise; - e.data = {details}; + if (details !== null) { + const data = {details}; + this._assignErrorData(e, data); + } throw e; } finally { this._removeWebRequestEventListeners(eventListeners); @@ -295,4 +350,37 @@ class RequestBuilder { } return result; } + + _assignErrorData(error, data) { + try { + error.data = data; + } catch (e) { + // On Firefox, assigning DOMException.data can fail in certain contexts. + // https://bugzilla.mozilla.org/show_bug.cgi?id=1776555 + try { + Object.defineProperty(error, 'data', { + configurable: true, + enumerable: true, + writable: true, + value: data + }); + } catch (e2) { + // NOP + } + } + } + + static _joinUint8Arrays(items, totalLength) { + if (items.length === 1) { + const {array, length} = items[0]; + if (array.length === length) { return array; } + } + const result = new Uint8Array(totalLength); + let position = 0; + for (const {array, length} of items) { + result.set(array, position); + position += length; + } + return result; + } } diff --git a/ext/js/data/anki-note-builder.js b/ext/js/data/anki-note-builder.js index 53cc29f6..9691aadb 100644 --- a/ext/js/data/anki-note-builder.js +++ b/ext/js/data/anki-note-builder.js @@ -320,8 +320,8 @@ class AnkiNoteBuilder { if (injectAudio && dictionaryEntryDetails.type !== 'kanji') { const audioOptions = mediaOptions.audio; if (typeof audioOptions === 'object' && audioOptions !== null) { - const {sources, preferredAudioIndex} = audioOptions; - audioDetails = {sources, preferredAudioIndex}; + const {sources, preferredAudioIndex, idleTimeout} = audioOptions; + audioDetails = {sources, preferredAudioIndex, idleTimeout}; } } if (injectScreenshot) { diff --git a/ext/js/data/options-util.js b/ext/js/data/options-util.js index a163580f..8c50493a 100644 --- a/ext/js/data/options-util.js +++ b/ext/js/data/options-util.js @@ -468,7 +468,8 @@ class OptionsUtil { {async: false, update: this._updateVersion16.bind(this)}, {async: false, update: this._updateVersion17.bind(this)}, {async: false, update: this._updateVersion18.bind(this)}, - {async: false, update: this._updateVersion19.bind(this)} + {async: false, update: this._updateVersion19.bind(this)}, + {async: false, update: this._updateVersion20.bind(this)} ]; if (typeof targetVersion === 'number' && targetVersion < result.length) { result.splice(targetVersion); @@ -975,4 +976,13 @@ class OptionsUtil { } return options; } + + _updateVersion20(options) { + // Version 20 changes: + // Added anki.downloadTimeout. + for (const profile of options.profiles) { + profile.options.anki.downloadTimeout = 0; + } + return options; + } } diff --git a/ext/js/display/display-anki.js b/ext/js/display/display-anki.js index 12133ad0..e2101481 100644 --- a/ext/js/display/display-anki.js +++ b/ext/js/display/display-anki.js @@ -48,6 +48,7 @@ class DisplayAnki { this._screenshotQuality = 100; this._scanLength = 10; this._noteGuiMode = 'browse'; + this._audioDownloadIdleTimeout = null; this._noteTags = []; this._modeOptions = new Map(); this._dictionaryEntryTypeModeMap = new Map([ @@ -133,7 +134,19 @@ class DisplayAnki { _onOptionsUpdated({options}) { const { general: {resultOutputMode, glossaryLayoutMode, compactTags}, - anki: {tags, duplicateScope, duplicateScopeCheckAllModels, suspendNewCards, checkForDuplicates, displayTags, kanji, terms, noteGuiMode, screenshot: {format, quality}}, + anki: { + tags, + duplicateScope, + duplicateScopeCheckAllModels, + suspendNewCards, + checkForDuplicates, + displayTags, + kanji, + terms, + noteGuiMode, + screenshot: {format, quality}, + downloadTimeout + }, scanning: {length: scanLength} } = options; @@ -150,6 +163,7 @@ class DisplayAnki { this._scanLength = scanLength; this._noteGuiMode = noteGuiMode; this._noteTags = [...tags]; + this._audioDownloadIdleTimeout = (Number.isFinite(downloadTimeout) && downloadTimeout > 0 ? downloadTimeout : null); this._modeOptions.clear(); this._modeOptions.set('kanji', kanji); this._modeOptions.set('term-kanji', terms); @@ -536,7 +550,7 @@ class DisplayAnki { const fields = Object.entries(modeOptions.fields); const contentOrigin = this._display.getContentOrigin(); const details = this._ankiNoteBuilder.getDictionaryEntryDetailsForNote(dictionaryEntry); - const audioDetails = (details.type === 'term' ? this._displayAudio.getAnkiNoteMediaAudioDetails(details.term, details.reading) : null); + const audioDetails = this._getAnkiNoteMediaAudioDetails(details); const optionsContext = this._display.getOptionsContext(); const {note, errors, requirements: outputRequirements} = await this._ankiNoteBuilder.createNote({ @@ -586,6 +600,12 @@ class DisplayAnki { return {text, offset}; } + _getAnkiNoteMediaAudioDetails(details) { + if (details.type !== 'term') { return null; } + const {sources, preferredAudioIndex} = this._displayAudio.getAnkiNoteMediaAudioDetails(details.term, details.reading); + return {sources, preferredAudioIndex, idleTimeout: this._audioDownloadIdleTimeout}; + } + // View note functions _onViewNoteButtonClick(e) { diff --git a/ext/js/media/audio-downloader.js b/ext/js/media/audio-downloader.js index 0991d14d..4142e3f4 100644 --- a/ext/js/media/audio-downloader.js +++ b/ext/js/media/audio-downloader.js @@ -18,6 +18,7 @@ /* global * JsonSchema * NativeSimpleDOMParser + * RequestBuilder * SimpleDOMParser * StringUtil */ @@ -50,7 +51,7 @@ class AudioDownloader { return []; } - async downloadTermAudio(sources, preferredAudioIndex, term, reading) { + async downloadTermAudio(sources, preferredAudioIndex, term, reading, idleTimeout) { const errors = []; for (const source of sources) { let infoList = await this.getTermAudioInfoList(source, term, reading); @@ -61,7 +62,7 @@ class AudioDownloader { switch (info.type) { case 'url': try { - return await this._downloadAudioFromUrl(info.url, source.type); + return await this._downloadAudioFromUrl(info.url, source.type, idleTimeout); } catch (e) { errors.push(e); } @@ -241,21 +242,42 @@ class AudioDownloader { return url.replace(/\{([^}]*)\}/g, (m0, m1) => (Object.prototype.hasOwnProperty.call(data, m1) ? `${data[m1]}` : m0)); } - async _downloadAudioFromUrl(url, sourceType) { + async _downloadAudioFromUrl(url, sourceType, idleTimeout) { + let signal; + let onProgress = null; + let idleTimer = null; + if (typeof idleTimeout === 'number') { + const abortController = new AbortController(); + ({signal} = abortController); + const onIdleTimeout = () => { + abortController.abort('Idle timeout'); + }; + onProgress = (done) => { + clearTimeout(idleTimer); + idleTimer = done ? null : setTimeout(onIdleTimeout, idleTimeout); + }; + idleTimer = setTimeout(onIdleTimeout, idleTimeout); + } + const response = await this._requestBuilder.fetchAnonymous(url, { method: 'GET', mode: 'cors', cache: 'default', credentials: 'omit', redirect: 'follow', - referrerPolicy: 'no-referrer' + referrerPolicy: 'no-referrer', + signal }); if (!response.ok) { throw new Error(`Invalid response: ${response.status}`); } - const arrayBuffer = await response.arrayBuffer(); + const arrayBuffer = await RequestBuilder.readFetchResponseArrayBuffer(response, onProgress); + + if (idleTimer !== null) { + clearTimeout(idleTimer); + } if (!await this._isAudioBinaryValid(arrayBuffer, sourceType)) { throw new Error('Could not retrieve audio'); diff --git a/ext/settings.html b/ext/settings.html index a3cc40f5..4dfa3100 100644 --- a/ext/settings.html +++ b/ext/settings.html @@ -1763,7 +1763,31 @@ - +
+
+
+
Idle download timeout (in milliseconds)
+
+ The maximum time before an idle download will be cancelled; 0 = no limit. + More… +
+
+
+ +
+
+ +
Suspend new cards
diff --git a/test/test-options-util.js b/test/test-options-util.js index 16660fd0..af64e7dd 100644 --- a/test/test-options-util.js +++ b/test/test-options-util.js @@ -470,7 +470,8 @@ function createProfileOptionsUpdatedTestData1() { fieldTemplates: null, suspendNewCards: false, noteGuiMode: 'browse', - apiKey: '' + apiKey: '', + downloadTimeout: 0 }, sentenceParsing: { scanExtent: 200, @@ -619,7 +620,7 @@ function createOptionsUpdatedTestData1() { } ], profileCurrent: 0, - version: 19, + version: 20, global: { database: { prefixWildcardsSupported: false -- cgit v1.2.3