Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-1644: Fix Unchecked runtime.lastError in message handler #362

Merged
merged 7 commits into from Apr 2, 2019
Next
consolidate and refactor sendMessage methods
  • Loading branch information
christophertino committed Mar 29, 2019
commit c19dc8952ce9b8741e63ac114b544e1eb5a08588
@@ -34,7 +34,7 @@ export default (state = initialState, action) => {
switch (action.type) {
case UPDATE_PANEL_DATA: {
const { account } = action.data;
if (account === null) {
if (!account) {
return Object.assign({}, initialState);
}
const {
@@ -15,18 +15,15 @@
*/
/* eslint no-use-before-define: 0 */

import globals from '../../../src/classes/Globals';
import { log } from '../../../src/utils/common';
import { sendMessageInPromise as panelSendPromiseMessage, sendMessage as panelSendMessage } from '../../panel/utils/msg';

/**
* Message wrapper function
* @param {string} origin content script origin
* @return {Object} set of APIs for handling messages
*/
export default function (origin) {
const IS_EDGE = (globals.BROWSER_INFO.name === 'edge');
const { onMessage } = chrome.runtime;

/**
* Send a message wrapped in a promise.
* @memberOf ContentScriptUtils
@@ -35,49 +32,8 @@ export default function (origin) {
* @param {Object} message message data
* @return {Promise} response or null
*/
let MESSAGE_ID = 0;
let LISTENER_ADDED = false;
function sendMessageInPromise(name, message) {
// On Edge 39.14965.1001.0 callback is not called when multiple
// Edge instances running. So instead we shoot message back
// from background. See onMessageHandler, HANDLE UNIVERSAL EVENTS HERE
// in src/background.js.To be removed, once Edge fixed.
if (IS_EDGE) {
return new Promise((resolve) => {
const messageId = MESSAGE_ID.toString();
MESSAGE_ID++;
if (!LISTENER_ADDED) {
LISTENER_ADDED = true;
onMessage.addListener((request, sender, sendResponse) => {
if (messageId === request.name) {
resolve(request.message);
}
if (sendResponse) {
sendResponse();
}
});
}
chrome.runtime.sendMessage({
name,
message,
origin,
messageId,
}, () => {});
});
}
return new Promise(((resolve) => {
chrome.runtime.sendMessage({
name,
message,
origin,
}, (response) => {
if (chrome.runtime.lastError) {
log(chrome.runtime.lastError, origin, name, message);
resolve(null);
}
resolve(response);
});
}));
panelSendPromiseMessage(name, message, origin);
}

/**
@@ -91,7 +47,7 @@ export default function (origin) {
*/
function sendMessage(name, message, callback) {
log(`origin ${origin} sending to handler`, name);
_sendMessageToHandler(name, origin, message, callback);
panelSendMessage(name, message, origin, callback);
}

/**
@@ -105,30 +61,7 @@ export default function (origin) {
*/
function sendMessageToBackground(name, message, callback) {
log(`origin ${origin} sending to background onMessageHandler`, name);
_sendMessageToHandler(name, '', message, callback);
}

/**
* Send a message without an `origin` parameter. This will
* be picked up by the general onMessageHandler in src/background.
*
* @private
*
* @param {string} name message name
* @param {string} source message origin
* @param {Object} message message data
* @param {function} [callback] callback called by the message recipient
*/
function _sendMessageToHandler(name, source, message, callback = function () {}) {
log(`_sendMessageToHandler:${source} sending to background`, name);
// @EDGE chrome.runtime.sendMessage(message) works, but
// const callback; chrome.runtime.sendMessage(message, callback) fails to
// execute and chrome.runtime.lastError is undefined.
chrome.runtime.sendMessage({
name,
message,
origin: source, // prevents eslint no-shadow
}, callback);
panelSendMessage(name, message, '', callback);
}

return {
@@ -141,7 +141,7 @@ class Rewards extends React.Component {
origin: 'rewards-hub',
type: 'action-signal',
};
sendMessage('setPanelData', { enable_offers: !enable_offers, signal }, undefined, 'rewardsPanel');
sendMessage('setPanelData', { enable_offers: !enable_offers, signal }, 'rewardsPanel');
sendMessage('ping', enable_offers ? 'rewards_on' : 'rewards_off');
// TODO catch
}
@@ -96,7 +96,7 @@ class Settings extends React.Component {
origin: 'rewards-hub',
type: 'action-signal',
};
sendMessage('setPanelData', { enable_offers: event.currentTarget.checked, signal }, undefined, 'rewardsPanel');
sendMessage('setPanelData', { enable_offers: event.currentTarget.checked, signal }, 'rewardsPanel');
sendMessage('ping', event.currentTarget.checked ? 'rewards_on' : 'rewards_off');
}
this.props.actions.toggleCheckbox({
@@ -27,17 +27,18 @@ const IS_EDGE = (globals.BROWSER_INFO.name === 'edge');
* @return {Promise}
*/
let MESSAGE_ID = 0;
const LISTENER_ADDED = false;
let LISTENER_ADDED = false;
export function sendMessageInPromise(name, message, origin = '') {
// On Edge 39.14965.1001.0 callback is not called when multiple
// Edge instances running. So instead we shoot message back
// Edge instances are running. So instead we pass the message back
// from background. See onMessageHandler, HANDLE UNIVERSAL EVENTS HERE
// in src/background.js.To be removed, once Edge fixed.
// in src/background.js. To be removed, once Edge is fixed.
if (IS_EDGE) {
return new Promise((resolve) => {
const messageId = MESSAGE_ID.toString();
MESSAGE_ID++;
if (!LISTENER_ADDED) {
LISTENER_ADDED = true;
onMessage.addListener((request, sender, sendResponse) => {
if (messageId === request.name) {
resolve(request.message);
@@ -63,7 +64,7 @@ export function sendMessageInPromise(name, message, origin = '') {
}, (response) => {
if (chrome.runtime.lastError) {
log(chrome.runtime.lastError, name, message);
resolve(null);
resolve(false);
}
resolve(response);
});
@@ -77,16 +78,16 @@ export function sendMessageInPromise(name, message, origin = '') {
*
* @param {string} name message name
* @param {Object} message message data
* @param {string} origin message origin
* @param {function} callback callback message
* @return {Object} response
* @todo runtime.sendMessage does not return any value.
*/
export function sendMessage(name, message, callback = function () {}, origin = null) {
export function sendMessage(name, message, origin = '', callback = _defaultCallback()) {
log('Panel sendMessage: sending to background', name);
// @EDGE chrome.runtime.sendMessage(message) works, but
// const callback; chrome.runtime.sendMessage(message, callback) fails to execute and chrome.runtime.lastError is undefined.
// const fallback = function () {}; // Workaround for Edge. callback cannot be undefined.
// callback = callback || fallback;
// @EDGE chrome.runtime.sendMessage(message) works, but the `callback` of
// chrome.runtime.sendMessage(message, callback) fails to
// execute and chrome.runtime.lastError is undefined.
return chrome.runtime.sendMessage({
name,
message,
@@ -105,12 +106,8 @@ export function sendMessage(name, message, callback = function () {}, origin = n
* @return {Object} response
* @todo runtime.sendMessage does not return any value.
*/
export function sendRewardMessage(name, message, callback = function () {}) {
log('Panel sendMessage: sending to background', name);
// @EDGE chrome.runtime.sendMessage(message) works, but
// const callback; chrome.runtime.sendMessage(message, callback) fails to execute and chrome.runtime.lastError is undefined.
// const fallback = function () {}; // Workaround for Edge. callback cannot be undefined.
// callback = callback || fallback;
export function sendRewardMessage(name, message, callback = _defaultCallback()) {
log('Panel sendRewardMessage: sending to background', name);
return chrome.runtime.sendMessage({
name,
message,
@@ -139,3 +136,16 @@ export function openSupportPage() {
sendMessage('account.openSupportPage');
window.close();
}


/**
* Default callback handler for sendMessage. Allows us to handle
* 'Unchecked runtime.lastError: The message port closed before a response was received' errors.
* This occurs when the `chrome.runtime.onmessage` handler returns `false` with no `callback()`
* but `chrome.runtime.sendMessage` has been passed a default callback.
*/
function _defaultCallback() {
if (chrome.runtime.lastError) {
log('defaultCallback error:', chrome.runtime.lastError);
}
}
ProTip! Use n and p to navigate between commits in a pull request.