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

CHANGELOG.md

@@ -1,9 +1,13 @@
 ### GHOSTERY 8.3.2 (UNRELEASED)
 
++ Ghostery tracker panel now updates dynamically in real time!
 + Remove unsupported file types for Opera automated-review
 + Removed uneccesary files for slimmer production build
 + Sync account creation UI betweem intro hub, panel and signon-web
 + Performance improvments when browsing certian Google sites (gmail, maps)
++ Feature parity for Edge browser (Human Web, Rewards)
++ Clean up various errors thrown by content scripts and message handlers
++ Updated Readme (team members and open-source projects)
 + Minor UI tweaks
 
 ### GHOSTERY 8.3.1 (January 31, 2019)

app/content-scripts/notifications.js

@@ -539,7 +539,12 @@ const NotificationsContentScript = (function (win, doc) {
 
 				const fileReader = new FileReader();
 				fileReader.onload = (fileLoadedEvent) => {
-					const fallback = function () {}; // Workaround for Edge. callback cannot be undefined.
+					// Workaround for Edge. Callback cannot be undefined.
+					const fallback = () => {
+						if (chrome.runtime.lastError) {
+							log('showBrowseWindow error:', chrome.runtime.lastError);
+						}
+					};
 					chrome.runtime.sendMessage({
 						origin: 'notifications',
 						name: 'importFile',

app/content-scripts/utils/msg.js

@@ -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 {

app/panel/components/Rewards.jsx

@@ -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
 	}

app/panel/components/Settings.jsx

@@ -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({

app/panel/utils/msg.js

@@ -17,6 +17,18 @@ import { log } from '../../../src/utils/common';
 const { onMessage } = chrome.runtime;
 const IS_EDGE = (globals.BROWSER_INFO.name === 'edge');
 
+/**
+ * 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.
+ */
+const defaultCallback = () => {
+	if (chrome.runtime.lastError) {
+		log('defaultCallback error:', chrome.runtime.lastError);
+	}
+};
+
 /**
  * Send a message to the handlers in src/background wrapped in a
  * promise. This should be used for messages that require a callback.
@@ -27,17 +39,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);
@@ -52,7 +65,11 @@ export function sendMessageInPromise(name, message, origin = '') {
 				message,
 				messageId,
 				origin,
-			}, () => {});
+			}, () => {
+				if (chrome.runtime.lastError) {
+					log('sendMessageInPromise error:', chrome.runtime.lastError);
+				}
+			});
 		});
 	}
 	return new Promise(((resolve) => {
@@ -63,7 +80,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 +94,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 +122,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,

package.json

@@ -47,7 +47,7 @@
   "dependencies": {
     "@cliqz/adblocker": "0.3.1",
     "base64-js": "^1.2.1",
-    "browser-core": "https://s3.amazonaws.com/cdncliqz/update/edge/ghostery/v7.35/7.35.1.8ec615c.tgz",
+    "browser-core": "https://s3.amazonaws.com/cdncliqz/update/edge/ghostery/v7.35/7.35.1.acd5469.tgz",
     "classnames": "^2.2.5",
     "d3": "^5.7.0",
     "foundation-sites": "^6.4.4-rc1",

src/classes/ExtMessenger.js

@@ -28,7 +28,11 @@ export class ExtMessenger {
 	}
 
 	sendMessage(extensionId, message) {
-		chrome.runtime.sendMessage(extensionId, message, () => {});
+		chrome.runtime.sendMessage(extensionId, message, () => {
+			if (chrome.runtime.lastError) {
+				log('ExtMessenger sendMessage error:', chrome.runtime.lastError);
+			}
+		});
 	}
 }
 

src/utils/utils.js

@@ -27,6 +27,15 @@ import { log, objectEntries } from './common';
 const { BROWSER_INFO } = globals;
 const IS_FIREFOX = (BROWSER_INFO.name === 'firefox');
 
+/**
+ * Handle chrome.runtime.lastError messages
+ */
+const defaultCallback = () => {
+	if (chrome.runtime.lastError) {
+		log('defaultCallback error:', chrome.runtime.lastError);
+	}
+};
+
 /**
  * Send message to a specific tab ID.
  * @memberOf BackgroundUtils
@@ -36,7 +45,7 @@ const IS_FIREFOX = (BROWSER_INFO.name === 'firefox');
  * @param  {Object} 	message 	message data
  * @param  {function} 	callback	function to call (at most once) when you have a response
  */
-export function sendMessage(tab_id, name, message, callback = function () {}) {
+export function sendMessage(tab_id, name, message, callback = defaultCallback()) {
 	log(`BACKGROUND SENT ${name} TO TAB`);
 	chrome.tabs.sendMessage(tab_id, {
 		name,
@@ -54,7 +63,7 @@ export function sendMessage(tab_id, name, message, callback = function () {}) {
  * @param  {Object} 	message 	message data
  * @param  {function} 	callback 	function to call (at most once) when you have a response
  */
-export function sendMessageToFrame(tab_id, frame_id, name, message, callback = function () {}) {
+export function sendMessageToFrame(tab_id, frame_id, name, message, callback = defaultCallback()) {
 	log(`BACKGROUND SENT ${name} TO TAB ${tab_id} - FRAME ${frame_id}`);
 	chrome.tabs.sendMessage(tab_id, {
 		name,
@@ -72,7 +81,11 @@ export function sendMessageToFrame(tab_id, frame_id, name, message, callback = f
  */
 export function sendMessageToPanel(name, message) {
 	log('BACKGROUND SENDS MESSAGE TO PANEL', name);
-	chrome.runtime.sendMessage({ name,	message });
+	chrome.runtime.sendMessage({ name, message }, () => {
+		if (chrome.runtime.lastError) {
+			log('sendMessageToPanel error:', chrome.runtime.lastError);
+		}
+	});
 }
 
 /**

yarn.lock

@@ -1259,9 +1259,9 @@ brorand@^1.0.1:
   resolved "https://registry.yarnpkg.com/brorand/-/brorand-1.1.0.tgz#12c25efe40a45e3c323eb8675a0a0ce57b22371f"
   integrity sha1-EsJe/kCkXjwyPrhnWgoM5XsiNx8=
 
-"browser-core@https://s3.amazonaws.com/cdncliqz/update/edge/ghostery/v7.35/7.35.1.8ec615c.tgz":
+"browser-core@https://s3.amazonaws.com/cdncliqz/update/edge/ghostery/v7.35/7.35.1.acd5469.tgz":
   version "7.35.1"
-  resolved "https://s3.amazonaws.com/cdncliqz/update/edge/ghostery/v7.35/7.35.1.8ec615c.tgz#f3af23b470490c5eac352a74b468bca3817f3416"
+  resolved "https://s3.amazonaws.com/cdncliqz/update/edge/ghostery/v7.35/7.35.1.acd5469.tgz#758f63214056b5e133bfc1c8a007a0d22c1f28c5"
   dependencies:
     "@cliqz-oss/dexie" "^2.0.4"
     "@cliqz/adblocker" "^0.6.8"