Closed Bug 1647127 Opened 5 years ago Closed 5 years ago

Callback parameter from chrome.runtime.sendMessage is 'undefined' when there are two listeners

Categories

(WebExtensions :: Untriaged, defect)

77 Branch
defect

Tracking

(Not tracked)

RESOLVED INVALID

People

(Reporter: nshusterman, Unassigned)

Details

Attachments

(1 file)

1.29 KB, application/x-zip-compressed
Details

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/77.0.3865.75 Safari/537.36

Steps to reproduce:

  1. Create custom extension.
  2. In the content script page request data from background script using code:
    chrome.runtime.sendMessage(
    "CONTENT_SCRIPT_LOAD",
    (contentScriptParams: string) => {
    try {
    if (chrome.runtime.lastError == null) {
    if (contentScriptParams !== 'undefined') {
    initFunc(contentScriptParams);
    }
    }
    } catch (ee) {
    console.error("Failed handling parameters from background");
    }
    });
  1. In background script create two listeners using this code:
    first listener:
    private readonly onMessage = async (messageType: string, messageSender: chrome.runtime.MessageSender) => {
    if (messageType !== "MessageFromInjected") {
    return;
    }

     	this.sendMessage({messageType, message, sender});
     };
    

second listener:

	private readonly onMessage = (
		message: string,
		sender: chrome.runtime.MessageSender,
		sendResponse: (response: string) => void): boolean | void => {

		switch (message) {
			case "CONTENT_SCRIPT_LOAD":
				return this.onContentScriptLoad(message, sender, sendResponse);
			default:
				break;
		}
		return undefined;
	}
	
	private onContentScriptLoad(
		message: string,
		sender: chrome.runtime.MessageSender,
		sendResponse: (response: string) => void): boolean {

		// Send the response asynchronously, so that we don't block content.
		setTimeout(
			() => { this.onContentScriptLoadImpl("hi", sendResponse); },
			0);

		// Keep connection with content script opened.
		return true;
	}

	private onContentScriptLoadImpl(
		message: string,
		sendResponse: (response: string) => void): void {

		sendResponse(response);
		console.info("Sent message to content", response);
	}

Actual results:

  1. The first listener is called first and immediately after it's return, the callback parameter from chrome.runtime.sendMessage is 'undefined'.
  2. "Failed handling parameters from background" is logged in the console.
  3. Only after that - sendResponse(response) is called in the second listener.

Expected results:

Even if the first listener is called and returns, the callback parameter from chrome.runtime.sendMessage callback parameter should be set after sendResponse(response) is called in the second listener.

Note-
if there is no first listener, the code works as expected.

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Product: Firefox → WebExtensions

Can you please provide more information? Which version of Firefox did you try this in? Is this a regression? Does this work in Chrome?

Also, this looks like Typescript, and part of a larger framework, and thus hard to figure out what's going on. Can you please provide a simple extension as a .zip file that can be loaded in Firefox directly?

(In reply to Natalia from comment #0)

  				if (contentScriptParams !== 'undefined') {

This sure looks wrong, though it's hard to follow if it's intentional.

Flags: needinfo?(nshusterman)

Hi,
Thanks for the quick response.
I used Firefox Version 77.0.1. It does work in Chrome and I don't know if its a regression.
I tried to create a simple extension to reproduce the problem, but unfortunately it was not reproduced there.

Flags: needinfo?(nshusterman)

Hello,

We’ve attempted to create an extension with the scripts posted in Comment 1 to try and reproduce the issue, however we did not manage.
Could you please attach the test extension mentioned in Comment 3 so we can try to reproduce the bug on our end as well?

Thank you !

Flags: needinfo?(nshusterman)
Attached file small extension

I'm attaching a small extension with two listeners in the background to a message from the content script.
Please note, the problem is not reproducible in this extension.

The code I added in my first comment is a part of a larger framework, there - the first listener is called and returned and immediately after that the callback parameter from chrome.runtime.sendMessage is 'undefined'.

This behavior works as expected in Chrome.

Flags: needinfo?(nshusterman)

Sure.
I'm attached the extension. It has two listeners in the background to a message from the content script.
Please note, the problem is not reproducible in this extension.
(To reproduce it I would expect after the first listener returns - the callback parameter from chrome.runtime.sendMessage to be 'undefined', even though the second listener calls sendResponse("response"), but in this extension the second listener calls to sendResponse() and the callback parameter is not 'undefined').

The code I added in my first comment is a part of a larger framework, there - the first listener is called and returned and immediately after that the callback parameter from chrome.runtime.sendMessage is 'undefined', even though the second listener calls sendResponse(response).

This behavior works as expected in Chrome.

(In reply to Natalia from comment #0)

  private readonly onMessage = async (messageType: string, messageSender: chrome.runtime.MessageSender) => {
  	if (messageType !== "MessageFromInjected") {
  		return;
  	}

  	this.sendMessage({messageType, message, sender});
  };

You're using an async function where you don't need one. Async functions always return a Promise, and we interpret that as a response to send back to the message sender. Since your method doesn't return anything, an implicit promise resolution of undefined is returned as a response. This is working as expected, see MDN for more details:
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/runtime/onMessage

Status: UNCONFIRMED → RESOLVED
Closed: 5 years ago
Resolution: --- → INVALID

Thank you!

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: