Skip to content

Commit

Permalink
Improve error reporting
Browse files Browse the repository at this point in the history 
Throw more descriptive errors, associate them better with the translation requests, and report them in the UI
  • Loading branch information
@jelmervdl
jelmervdl committed Jul 14, 2022
1 parent c2b045c commit 17cc659
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 46 deletions.
14 changes: 10 additions & 4 deletions extension/controller/backgroundScript.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ async function detectLanguage({sample, suggested}, provider) {
const State = {
PAGE_LOADING: 'page-loading',
PAGE_LOADED: 'page-loaded',
PAGE_ERROR: 'page-error',
TRANSLATION_NOT_AVAILABLE: 'translation-not-available',
TRANSLATION_AVAILABLE: 'translation-available',
DOWNLOADING_MODELS: 'downloading-models',
Expand Down Expand Up @@ -350,8 +351,6 @@ const state = {
developer: false // should we show the option to record page translation requests?
};

state.provider = 'translatelocally'; // For testing in Chrome

// State per tab
const tabs = new Map();

Expand Down Expand Up @@ -495,6 +494,11 @@ function connectContentScript(contentScript) {
? State.TRANSLATION_AVAILABLE
: State.TRANSLATION_NOT_AVAILABLE
}));
}).catch(error => {
tab.update(state => ({
state: State.PAGE_ERROR,
error
}));
});
break;

Expand Down Expand Up @@ -529,8 +533,10 @@ function connectContentScript(contentScript) {
if (e && e.message && e.message === 'removed by filter' && e.request && e.request._abortSignal.aborted)
return;

// rethrow any other error
throw e;
tab.update(state => ({
state: State.TRANSLATION_ERROR,
error: e.message
}));
})
.finally(() => {
tab.update(state => ({
Expand Down
95 changes: 54 additions & 41 deletions extension/controller/translation/WASMTranslationHelper.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,26 +227,30 @@ class WorkerChannel {
entries.sort(({model: a}, {model: b}) => (a.shortName.indexOf('tiny') === -1 ? 1 : 0) - (b.shortName.indexOf('tiny') === -1 ? 1 : 0));

if (!entries)
throw new Error(`No model for ${from} -> ${to}`);
throw new Error(`No model for '${from}' -> '${to}'`);

const entry = first(entries).model;

const compressedArchive = await this.getItemFromCacheOrWeb(entry.url, undefined, entry.checksum);
const compressedArchive = await this.getItemFromCacheOrWeb(entry.url, entry.checksum);

const archive = pako.inflate(compressedArchive);

const files = await untar(archive.buffer);

// Find the config yml file
const configFile = files.find(file => file.name.endsWith('config.intgemm8bitalpha.yml'));
const find = (filename) => {
const found = files.find(file => file.name.match(/(?:^|\/)([^\/]+)$/)[1] === filename)
if (found === undefined)
throw new Error(`Could not find '${filename}' in model archive`);
return found;
};

const config = YAML.parse(configFile.readAsString());
const config = YAML.parse(find('config.intgemm8bitalpha.yml').readAsString());

const model = files.find(file => file.name.endsWith(config.models[0])).buffer;
const model = find(config.models[0]).buffer;

const vocabs = config.vocabs.map(vocab => files.find(file => file.name.endsWith(vocab)).buffer);
const vocabs = config.vocabs.map(vocab => find(vocab).buffer);

const shortlist = files.find(file => file.name.endsWith(config.shortlist[0])).buffer;
const shortlist = find(config.shortlist[0]).buffer;

performance.measure('loadTranslationModel', `loadTranslationModule.${JSON.stringify({from, to})}`);

Expand All @@ -261,24 +265,25 @@ class WorkerChannel {
* @param {String} checksum sha256 checksum as hexadecimal string
* @returns {Promise<ArrayBuffer>}
*/
async getItemFromCacheOrWeb(url, size, checksum) {
const cache = await caches.open(CACHE_NAME);
const match = await cache.match(url);

// It's not already in the cache? Then return the downloaded version
// (but also put it in the cache)
if (!match)
return this.getItemFromWeb(url, size, checksum, cache);

// Found it in the cache, let's check whether it (still) matches the
// checksum.
const buffer = await match.arrayBuffer();
if (await this.digestSha256AsHex(buffer) !== checksum) {
cache.delete(url);
throw new Error("Error downloading translation engine. (checksum)")
}
async getItemFromCacheOrWeb(url, checksum) {
try {
const cache = await caches.open(CACHE_NAME);
const match = await cache.match(url);

if (match) {
// Found it in the cache, let's check whether it (still) matches the
// checksum. If not, redownload it.
const buffer = await match.arrayBuffer();
if (await this.digestSha256AsHex(buffer) === checksum)
return buffer;
else
cache.delete(url);
}

return buffer;
return await this.getItemFromWeb(url, checksum, cache);
} catch (e) {
throw new Error(`Failed to download '${url}': ${e.message}`);
}
}

/**
Expand All @@ -289,7 +294,7 @@ class WorkerChannel {
* @param {Cache?} cache optional cache to save response into
* @returns {Promise<ArrayBuffer>}
*/
async getItemFromWeb(url, size, checksum, cache) {
async getItemFromWeb(url, checksum, cache) {
try {
// Rig up a timeout cancel signal for our fetch
const abort = new AbortController();
Expand Down Expand Up @@ -398,7 +403,7 @@ class WorkerChannel {
);

if (!shared.size)
throw new Error(`No model available to translate from ${from} to ${to}`);
throw new Error(`No model available to translate from '${from}' to '${to}'`);

return [
outbound.find(model => shared.has(model.to)),
Expand Down Expand Up @@ -442,7 +447,11 @@ class WorkerChannel {

// Put this worker to work, marking as busy
worker.idle = false;
await this.consumeBatch(batch, worker.worker);
try {
await this.consumeBatch(batch, worker.worker);
} catch (e) {
batch.requests.forEach(({reject}) => reject(e));
}
worker.idle = true;

// Is there more work to be done? Do another idleRequest
Expand All @@ -469,19 +478,23 @@ class WorkerChannel {
const {from, to, html, priority} = request;

return new Promise(async (resolve, reject) => {
// Batching key: only requests with the same key can be batched
// together. Think same translation model, same options.
const key = JSON.stringify({from, to});

// (Fetching models first because if we would do it between looking
// for a batch and making a new one, we end up with a race condition.)
const models = await this.getModels(from, to);

// Put the request and its callbacks into a fitting batch
this.enqueue({key, models, request, resolve, reject, priority});

// Tell a worker to pick up the work at some point.
this.notify();
try {
// Batching key: only requests with the same key can be batched
// together. Think same translation model, same options.
const key = JSON.stringify({from, to});

// (Fetching models first because if we would do it between looking
// for a batch and making a new one, we end up with a race condition.)
const models = await this.getModels(from, to);

// Put the request and its callbacks into a fitting batch
this.enqueue({key, models, request, resolve, reject, priority});

// Tell a worker to pick up the work at some point.
this.notify();
} catch (e) {
reject(e);
}
});
}

Expand Down
6 changes: 5 additions & 1 deletion extension/view/static/popup.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,11 @@
</div>
<div data-state="translation-error">
<p>Error during translation:</p>
<p><code id="error-message" data-bind:text-content="error"></code></p>
<p><code data-bind:text-content="error"></code></p>
</div>
<div data-state="page-error">
<p>Error:</p>
<p><code data-bind:text-content="error"></code></p>
</div>
<div data-state="page-loading">
<!-- tab noticed by webRequest, but its content-script didn't connect -->
Expand Down

0 comments on commit 17cc659

Please sign in to comment.