Skip to content
This repository has been archived by the owner on Feb 25, 2023. It is now read-only.

Commit

Permalink
Audio download timeout (#2187)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
toasted-nutbread authored Aug 20, 2022
1 parent 02483a4 commit 310303c
Show file tree
Hide file tree
Showing 11 changed files with 230 additions and 23 deletions.
8 changes: 8 additions & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,14 @@
"rules": {
"no-implicit-globals": "error"
}
},
{
"files": [
"ext/js/**/*.js"
],
"globals": {
"AbortController": "readonly"
}
}
]
}
8 changes: 7 additions & 1 deletion ext/data/schemas/options-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -878,7 +878,8 @@
"suspendNewCards",
"displayTags",
"noteGuiMode",
"apiKey"
"apiKey",
"downloadTimeout"
],
"properties": {
"enable": {
Expand Down Expand Up @@ -1002,6 +1003,11 @@
"apiKey": {
"type": "string",
"default": ""
},
"downloadTimeout": {
"type": "number",
"default": 0,
"minimum": 0
}
}
},
Expand Down
12 changes: 12 additions & 0 deletions ext/issues.html
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,18 @@ <h2 id="audio-download-failed">Audio download failed due to possible extension p
</div></div></div></div>
</div>

<h2 id="audio-download-idle-timeout">Audio download was cancelled due to an idle timeout</h2>
<div class="settings-group">
<div class="settings-item"><div class="settings-item-inner"><div class="settings-item-left"><div class="settings-item-label">
<p>
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 <em>Idle download timeout</em> setting on the <a href="/settings.html#!anki">settings page</a>
specifies a time limit for stalled downloads.
</p>
</div></div></div></div>
</div>

<div class="footer-padding"></div>

</div>
Expand Down
32 changes: 24 additions & 8 deletions ext/js/background/backend.js
Original file line number Diff line number Diff line change
Expand Up @@ -1809,15 +1809,16 @@ class Backend {
return null;
}

const {sources, preferredAudioIndex} = details;
const {sources, preferredAudioIndex, idleTimeout} = details;
let data;
let contentType;
try {
({data, contentType} = await this._audioDownloader.downloadTermAudio(
sources,
preferredAudioIndex,
term,
reading
reading,
idleTimeout
));
} catch (e) {
const error = this._getAudioDownloadError(e);
Expand Down Expand Up @@ -1918,26 +1919,41 @@ 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; }
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,
referenceUrl: '/issues.html#audio-download-failed'
};
return result;
return this._createAudioDownloadError('Audio download failed due to possible extension permissions error', 'audio-download-failed', errors);
}
}
}
}
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;

Expand Down
90 changes: 89 additions & 1 deletion ext/js/background/request-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
}
4 changes: 2 additions & 2 deletions ext/js/data/anki-note-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
12 changes: 11 additions & 1 deletion ext/js/data/options-util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
}
24 changes: 22 additions & 2 deletions ext/js/display/display-anki.js
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand Down Expand Up @@ -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;

Expand All @@ -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);
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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) {
Expand Down
32 changes: 27 additions & 5 deletions ext/js/media/audio-downloader.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
/* global
* JsonSchema
* NativeSimpleDOMParser
* RequestBuilder
* SimpleDOMParser
* StringUtil
*/
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}
Expand Down Expand Up @@ -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');
Expand Down
Loading

0 comments on commit 310303c

Please sign in to comment.