Skip to content

Commit

Permalink
Reject with real Errors like Firefox does
Browse files Browse the repository at this point in the history
  • Loading branch information
fregante authored and rpl committed Mar 11, 2021
1 parent fa4b26f commit 97acf68
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 8 deletions.
4 changes: 2 additions & 2 deletions src/browser-polyfill.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ if (typeof browser === "undefined" || Object.getPrototypeOf(browser) !== Object.
const makeCallback = (promise, metadata) => {
return (...callbackArgs) => {
if (extensionAPIs.runtime.lastError) {
promise.reject(extensionAPIs.runtime.lastError);
promise.reject(new Error(extensionAPIs.runtime.lastError.message));
} else if (metadata.singleCallbackArg ||
(callbackArgs.length <= 1 && metadata.singleCallbackArg !== false)) {
promise.resolve(callbackArgs[0]);
Expand Down Expand Up @@ -484,7 +484,7 @@ if (typeof browser === "undefined" || Object.getPrototypeOf(browser) !== Object.
if (extensionAPIs.runtime.lastError.message === CHROME_SEND_MESSAGE_CALLBACK_NO_RESPONSE_MESSAGE) {
resolve();
} else {
reject(extensionAPIs.runtime.lastError);
reject(new Error(extensionAPIs.runtime.lastError.message));
}
} else if (reply && reply.__mozWebExtensionPolyfillReject__) {
// Convert back the JSON representation of the error into
Expand Down
17 changes: 16 additions & 1 deletion test/fixtures/detect-existing-browser-api-object/content.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* global originalAPIObjects */

test("browser api object in content script", (t) => {
test("browser api object in content script", async (t) => {
t.ok(browser && browser.runtime, "a global browser API object should be defined");
t.ok(chrome && chrome.runtime, "a global chrome API object should be defined");

Expand All @@ -10,12 +10,27 @@ test("browser api object in content script", (t) => {
// On Firefox, window is not the global object for content scripts, and so we expect window.browser to not
// be defined.
t.equal(window.browser, undefined, "window.browser is expected to be undefined on Firefox");

try {
await browser.storage.sync.set({a: 'a'.repeat(10000000)});
t.fail('It should throw when attempting to set an object over quota');
} catch (error) {
t.ok(error instanceof Error);
}
} else {
// Check that the polyfill has created a wrapped API namespace as expected.
t.notEqual(browser.runtime, chrome.runtime, "browser.runtime and chrome.runtime should not be equal");
// On chrome, window is the global object and so the polyfilled browser API should
// be also equal to window.browser.
t.equal(browser, window.browser, "browser and window.browser should be the same object");

chrome.storage.local.set({a: 'a'.repeat(10000000)}, () => {
t.ok(chrome.runtime.lastError, 'It should throw when attempting to set an object over quota');
t.equal(chrome.runtime.lastError.message, 'QUOTA_BYTES quota exceeded');
t.notOk(chrome.runtime.lastError instanceof Error);
});

t.end();
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,7 @@
]
}
],
"permissions": []
"permissions": [
"storage"
]
}
11 changes: 7 additions & 4 deletions test/test-async-functions.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"use strict";

const {deepEqual, equal, fail, ok, throws} = require("chai").assert;
const {deepEqual, equal, fail, ok, throws, instanceOf} = require("chai").assert;
const sinon = require("sinon");

const {setupTestDOMWindow} = require("./setup");
Expand Down Expand Up @@ -56,7 +56,7 @@ describe("browser-polyfill", () => {
it("rejects the returned promise if chrome.runtime.lastError is not null", () => {
const fakeChrome = {
runtime: {
lastError: new Error("fake lastError"),
lastError: {message: "fake lastError"},
},
tabs: {
query: sinon.stub(),
Expand All @@ -70,8 +70,11 @@ describe("browser-polyfill", () => {

return window.browser.tabs.query({active: true}).then(
() => fail("Expected a rejected promise"),
(err) => equal(err, fakeChrome.runtime.lastError,
"Got the expected error in the rejected promise")
(err) => {
instanceOf(err, window.Error, "Expected the error to be an instance of Error");
equal(err.message, fakeChrome.runtime.lastError.message,
"Got the expected error in the rejected promise");
}
);
});
});
Expand Down

0 comments on commit 97acf68

Please sign in to comment.