webextension-polyfill: runtime.onMessage is broken

Just spent a good hour figuring this out for myself. Unfortunately different documentation on MDN seems to disagree how this this is supposed to work:

runtime.onMessage (which should probably be considered the source of truth) shows this working with a sendResponse callback, returning a boolean indicating if the callback will be invoked asynchronously.

function handleMessage(request, sender, sendResponse) {  
  console.log(`content script sent a message: ${request.content}`);
  setTimeout(() => {
    sendResponse({response: "async response from background script"});
  }, 1000);  
  return true;
}

browser.runtime.onMessage.addListener(handleMessage);

tabs.sendMessage shows this working by returning a promise.

browser.runtime.onMessage.addListener(request => {
  console.log("Message from the background script:");
  console.log(request.greeting);
  return Promise.resolve({response: "Hi from content script"});
});

Hi there from the future. Since this is the issue linked to from MDN, I decided to post my discovery here. For any Rust folks trying to bind to browser.runtime.onMessage.addListener properly, here’s how to do it:

#[wasm_bindgen]
extern "C" {
    #[wasm_bindgen(js_namespace = ["browser", "runtime"], js_name = "sendMessage")]
    pub async fn send_message(message: JsValue) -> JsValue;

    #[wasm_bindgen(js_namespace = ["browser", "runtime", "onMessage"], js_name = "addListener")]
    pub fn add_listener(handle: &Closure<dyn Fn(JsValue, JsValue) -> Promise>);
}

For the pub async fn send_message, make sure you have wasm-bindgen-futures as a dependency. It auto-converts JS Promises into Rust’s async system.

Notice that we’re only accepting two arguments to the closure passed to add_listener. We completely ignore the sendMessage argument and force a Promise to be returned, as per the MDN docs.

Downstream in our background script, we can add the listener like so:

    let handle_msg = Closure::wrap(Box::new(move |msg, sender| {
        Promise::resolve(&JsValue::from_str("From background!"))
    }) as Box<dyn Fn(JsValue, JsValue) -> Promise>);
    add_listener(&handle_msg); // The binding we just wrote.
    handle_msg.forget();

You’re free to do pretty much whatever you want inside the Box’d closure, including moving in Rust structures, write log messages, etc. As long as a Promise is the return value. The final forget is also critical to ensure that the closure isn’t dropped on the Rust side before it has a chance to be used on the JS side.

Cheers!

So is somebody gonna edit the MDN? I will do it myself if nobody else will. The question is whether I mark the sendResponse deprecated or just add a NOTE about future plans of W3C and about current behavior of popular webextension-polyfill library.

EDIT: One more thing, what about similar browser.runtime.onMessageExternal (MDN )? Does it work with polyfill and does it share the same destiny?

EDIT 2: MDN is now updated with warning block.

My apologies for the late reply, I deferred closing this issue in the last bug triage meeting because I wanted to discuss it again with @kmaglione (mainly to collect additional details and then be able to provide a more complete comment here).

While discussing this issue with @kmaglione it did come out that the polyfill implementation of the runtime.onMessage API doesn’t provide the sendResponse callback on purpose: the reason is that the sendResponse callback is going to be removed from the W3C specs (https://browserext.github.io/browserext/#runtime, the callback is still listed in the current spec draft, but as far as I know, mainly what I did learn from the discussion with Kris, its removal has been discussed and it is going to be removed).

We agreed that this “incompatibility”, between the current Firefox implementation and the W3C spec for this API event, should be better highlighted and explained in the API documentation on MDN to prevent confusion about the implementation in the polyfill.

@6a68 With the polyfill, you can only send the response by returning a promise.

Here is what I do now:

1. There is no sendResponse function when using the polyfill.
2. To send a response, return a promise in the handler.
3. If there is no response but the message is handled, return false. Never return true or Firefox would expect you to call sendResponse function.
4. If the message is not handled, return undefined.

I think the proper “fix” to this issue is to remove/deprecate (since it is going to be removed) sendResponse on MDN and add a warning about the current state of polyfilled environment. Currently, the polyfill disagrees with MDN, which is confusing.

Also, I found that the document had been updated and the usage of the promise is now included (which was not when the issue was filed): https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/runtime/onMessage$compare?to=1204311&from=1178671

Currently I patch it myself. https://github.com/eight04/ansi-viewer/blob/8f6d33f6ee8713ca29d00075c94e9fff89ba04e9/lib/browser-polyfill.js#L828

So… is someone going to merge the PR?