1657384 - runtime messaging API does throw "Error: Conduits:RuntimeMessage: message reply cannot be cloned."

Status: NEW

(WebExtensions :: General, defect, P3)

Description

[Luca Greco [:rpl] [:luca] [:lgreco]]

This issue has been reported on the webextension-polyfill repo and on discourse by an addon developer:

• https://github.com/mozilla/webextension-polyfill/issues/235
• https://discourse.mozilla.org/t/latest-firefox-update-broke-add-on-sendmessage/65159

The issue is very likely triggered by the extension by trying to send an object that includes "non structure cloneable" values, but it is possible that we changed the behavior a bit in 79 and so we should double-check:

• if we were originally ignoring the non cloneable values and the error wasn't triggered at all
• if we should throw an error to the extension, "Conduits:RuntimeMessage" isn't supposed to be part of the message that the extension does receive (because it is actually an internal implementation detail).

We should mark this issue as "regression range wanted", but to do that we have to add to this issue:

• a minimal test extension that reproduce the issue (or at least a link to the existing extension that is already able to trigger it)
• some steps to reproduce the issue consistently

Comment 1

[Luca Greco [:rpl] [:luca] [:lgreco]]

Another extension developer commented on the webextension-polyfill issue and kindly provided us an STR, quoting from https://github.com/mozilla/webextension-polyfill/issues/235#issuecomment-669481339:

---

Hi @rpl, We are also experiencing issues with exactly this bug on our extension.

The extension is Anity

Steps to reproduce:

1. Firefox browser version 79.0
2. Go to japanese comics image: example image
3. Click the Anity extension -> Click translate

Expected:
Loading bar appears at the end of which the text on the image should be translated to English

Actual:
Loading bar appears but the text is not translated (due to client error from bug)

We have also validated that this does not happen on different browsers and does not happen on versions 78.0 and lower.

Thank you very much for your help

Comment 2

[Alice0775 White]

Regression window:

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=e8d0f18c2300a223c31847d27196d1a8ad673515&tochange=ca3057f6cdc555f3ddfb1f10afa6805a43253a85

Comment 3

[Rob Wu [:robwu]]

The leak of the internal error message has been resolved by bug 1655624.

The reporter on Github provided some extra context in https://github.com/mozilla/webextension-polyfill/issues/235#issuecomment-670488605

@Rob--W The following API's were not working as expected following the Firefox 79 update:

• browser.runtime.sendMessage(); and browser.tabs.sendMessage(); aren't able to serialize messages to JSON that were previously serializable.
  Some examples:

  1. undefined now throws the aforementioned message reply cannot be cloned.
  2. axios responses that were previously serializable now throw a similar message.

• browser.runtime.sendMessage(); and browser.tabs.sendMessage(); - couldn't await on a response from them, even when the message was serializable. (I didn't dig too deep and made a temporary fix for 79 - passing and receiving messages one way)

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:mixedpuppy, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 5

[Luca Greco [:rpl] [:luca] [:lgreco]]

Hey Rob,
It looks like Bug 1655624 has been landed in 81 and also uplifted to 80, can this issue be also marked as fixed now?
(also, if that is the case then status-firefox80 may have to be set to fixed as in Bug 1655624)

Comment 6

[Rob Wu [:robwu]]

(In reply to Luca Greco [:rpl] [:luca] [:lgreco] from comment #0)

The issue is very likely triggered by the extension by trying to send an object that includes "non structure cloneable" values, but it is possible that we changed the behavior a bit in 79 and so we should double-check:

• if we were originally ignoring the non cloneable values and the error wasn't triggered at all

This is the case. STR:

browser.runtime.onMessage(() => Promise.resolve(new Headers()));

// Another extension page:
browser.runtime.sendMessage(1).then(console.log, console.error);

Before 79, the reply was {}.
Since 79, the reply is Error: Conduits:RuntimeMessage: message reply cannot be cloned.
Starting from 80, the reply will be Error: Could not establish connection. Receiving end does not exist. (the real error will be reported to the global JS console as [Exception... "Conduits:RuntimeMessage: message reply cannot be cloned." nsresult: "0x80004005 (DataCloneError)" location: "<unknown>" data: no]).

If we want to continue with this new behavior, then the bug can be closed.
Chrome also had our old behavior, so we can also consider fixing this.

• if we should throw an error to the extension, "Conduits:RuntimeMessage" isn't supposed to be part of the message that the extension does receive (because it is actually an internal implementation detail).

This part has been fixed by bug 1655624.

Comment 7

[Jim Mathies [:jimm]]

Hey Shane, can we close this one out (or take it out of tracking)?

Comment 8

[Tomislav Jovanovic :zombie]

I don't think this needs to be tracked.

This is only a regression insofar as we had a bug in how we were serializing onMessage responses. We're using Structured Cloning for every other place to forward extension-provided data, except for onMessage return value which is just returned as a result of the IPC message. Previously, with MessageManagers that had a fallback to JSON if standard serialization throws, which we don't actually want.

The fix here is to explicitly use Structured Cloning, and throw with a clear error message. Since we don't actually plan to change behavior here, I'm marking it as fix-optional for 82.

Comment 9

[Marco Castelluccio [:marco]]

Given the last comment, this doesn't seem a P1/S2. Shane, could you confirm and retriage?