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

Commit

Permalink
Fix up background error communication to the content worker
Browse files Browse the repository at this point in the history
Add .popupMessage to login and registration errors
Improve analytics around login/registration errors, and document the events
Re-raise the error when takeShot fails
Re-raise the error in catchPromise'd promises
Fixes #2433
  • Loading branch information
ianb committed Mar 28, 2017
1 parent b63d723 commit e0f6232
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 14 deletions.
20 changes: 17 additions & 3 deletions addon/webextension/background/auth.js
Expand Up @@ -50,10 +50,19 @@ window.auth = (function () {
resolve(true);
analytics.sendEvent("registered");
} else {
analytics.sendEvent("register-failed", `bad-response-${req.status}`);
console.warn("Error in response:", req.responseText);
reject(new Error("Bad response: " + req.status));
let exc = new Error("Bad response: " + req.status);
exc.popupMessage = "LOGIN_ERROR";
reject(exc);
}
});
req.onerror = catcher.watchFunction(() => {
analytics.sendEvent("register-failed", "connection-error");
let exc = new Error("Error contacting server");
exc.popupMessage = "LOGIN_CONNECTION_ERROR";
reject(exc);
});
req.send(uriEncode({
deviceId: registrationInfo.deviceId,
secret: registrationInfo.secret,
Expand All @@ -77,10 +86,14 @@ window.auth = (function () {
}
} else if (req.status >= 300) {
console.warn("Error in response:", req.responseText);
reject(new Error("Could not log in: " + req.status));
let exc = new Error("Could not log in: " + req.status);
exc.popupMessage = "LOGIN_ERROR";
analytics.sendEvent("login-failed", `bad-response-${req.status}`);
reject(exc);
} else if (req.status === 0) {
let error = new Error("Could not log in, server unavailable");
analytics.sendEvent("login-failed");
error.popupMessage = "LOGIN_CONNECTION_ERROR";
analytics.sendEvent("login-failed", "connection-error");
reject(error);
} else {
initialized = true;
Expand All @@ -96,6 +109,7 @@ window.auth = (function () {
}
});
req.onerror = catcher.watchFunction(() => {
analytics.sendEvent("login-failed", "connection-error");
let exc = new Error("Connection failed");
exc.url = loginUrl;
exc.popupMessage = "CONNECTION_ERROR";
Expand Down
5 changes: 3 additions & 2 deletions addon/webextension/background/communication.js
Expand Up @@ -22,15 +22,16 @@ window.communication = (function () {
result = func.apply(null, req.args);
} catch (e) {
console.error(`Error in ${req.funcName}:`, e, e.stack);
sendResponse({type: "error", name: e+""});
// FIXME: should consider using makeError from catcher here:
sendResponse({type: "error", message: e+""});
return;
}
if (result && result.then) {
result.then((concreteResult) => {
sendResponse({type: "success", value: concreteResult});
}, (errorResult) => {
console.error(`Promise error in ${req.funcName}:`, errorResult, errorResult && errorResult.stack);
sendResponse({type: "error", name: errorResult+""});
sendResponse({type: "error", message: errorResult+""});
});
return true;
} else {
Expand Down
5 changes: 3 additions & 2 deletions addon/webextension/background/takeshot.js
Expand Up @@ -47,8 +47,9 @@ window.takeshot = (function () {
return browser.tabs.update(openedTab.id, {url: shot.viewUrl});
}).then(() => {
return shot.viewUrl;
}).catch(() => {
return browser.tabs.remove(openedTab.id);
}).catch((error) => {
browser.tabs.remove(openedTab.id);
throw error;
}));
}));

Expand Down
1 change: 1 addition & 0 deletions addon/webextension/catcher.js
Expand Up @@ -58,6 +58,7 @@ window.catcher = (function () {
console.error("------Error in promise:", e+"");
console.error(e.stack);
exports.unhandled(makeError(e));
throw e;
});
};

Expand Down
4 changes: 0 additions & 4 deletions addon/webextension/clipboard.js
Expand Up @@ -4,10 +4,6 @@ window.clipboard = (function () {
let exports = {};

exports.copy = function (text) {
if(!text) {
catcher.unhandled(new Error("Screenshot could not be saved"));
return false;
}
let el = document.createElement("textarea");
document.body.appendChild(el);
el.value = text;
Expand Down
6 changes: 4 additions & 2 deletions addon/webextension/selector/callBackground.js
Expand Up @@ -5,10 +5,12 @@ window.callBackground = function (funcName, ...args) {
if (result.type === "success") {
return result.value;
} else if (result.type === "error") {
throw new Error(result.name);
let exc = new Error(result.message);
exc.name = "BackgroundError";
throw exc;
} else {
console.error("Unexpected background result:", result);
let exc = new Error("Bad response from background page");
let exc = new Error(`Bad response type from background page: ${result.type || undefined}`);
exc.resultType = result.type || "undefined";
throw exc;
}
Expand Down
5 changes: 5 additions & 0 deletions addon/webextension/selector/shooter.js
Expand Up @@ -88,6 +88,11 @@ window.shooter = (function () { // eslint-disable-line no-unused-vars
}).then((url) => {
const copied = window.clipboard.copy(url);
return callBackground("openShot", { url, copied });
}, (error) => {
if (error.name != "BackgroundError") {
// BackgroundError errors are reported in the Background page
throw error;
}
}).then(() => uicontrol.deactivate()));
};

Expand Down
6 changes: 5 additions & 1 deletion docs/METRICS.md
@@ -1,5 +1,5 @@

## Firefox Screenshots Metrics

*Last Update: 2017-03-21*

This document is a summary of the metrics Firefox Screenshots will record, how we're recording them, and what we're looking for in those metrics. There are two main areas we'll look at:
Expand Down Expand Up @@ -119,6 +119,10 @@ The primary change was in `server/src/pages/shot/share-buttons.js`
16. ~~Hit shot button on a page that can't be shot (XUL page) `addon/abort-start-shot/xul-page`~~
17. [ ] Hit shot button on a page that uses `<frameset>` and can't be shot, `addon/abort-start-shot/frame-page` (FIXME: todo)
99. Toggle shot button off `addon/cancel-shot/toolbar-button`
99. Bad response when trying to login `addon/login-failed/bad-response-CODE`
99. Connection error trying to login `addon/login-failed/connection-error`
99. Bad response when trying to register `addon/register-failed/bad-response-CODE`
99. Connection error trying to register `addon/register-failed/connection-error`

Deprecated:

Expand Down

0 comments on commit e0f6232

Please sign in to comment.