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

Inconsistency: runtime.onMessage Promise return #338

Open
erosman opened this issue Dec 20, 2022 · 5 comments
Open

Inconsistency: runtime.onMessage Promise return #338

erosman opened this issue Dec 20, 2022 · 5 comments
Labels
implemented: firefox Implemented in Firefox implemented: safari Implemented in Safari inconsistency Inconsistent behavior across browsers supportive: chrome Supportive from Chrome

Comments

@erosman
Copy link

erosman commented Dec 20, 2022

Chrome MV3 runtime.onMessage returns immediately instead of waiting for the promise to resolve.

popup.js (browser action)

async function send() {
  const response = await chrome.runtime.sendMessage({...});
  console.log('response called', response);
}

background.js (service worker)

chrome.runtime.onMessage.addListener(e => onMessage(e));

async function onMessage(message) {
  return fetch('https://example.com/')
  .then(response => response.json())
  .then(response => {
    console.log('json called', response);
    return response;
  })
  .catch(error => {
    console.log('error called', error);
  });
}

Result

response called undefined
json called {...}

See also:
Issue 1185241: Support Promise as return value from runtime.onMessage callback
Chrome mv3 await sendMessage to background service worker

@Rob--W Rob--W added inconsistency Inconsistent behavior across browsers follow-up: chrome Needs a response from a Chrome representative and removed needs-triage labels Jan 5, 2023
@xeenon xeenon added implemented: safari Implemented in Safari implemented: firefox Implemented in Firefox labels Jan 19, 2023
@artisticfox8
Copy link

I think I came across this in Firefox too. I had to implement using sendResponse and return true for async calls before sending response, instead of using await

@rdcronin rdcronin added supportive: chrome Supportive from Chrome and removed follow-up: chrome Needs a response from a Chrome representative labels Sep 13, 2023
@rdcronin
Copy link
Contributor

I'm supportive of this in Chrome (though it's not yet implemented).

To clarify, returning a Promise from a message listener would keep the message pipe open until the promise is resolved, at which point it passes the resolved value back to the caller.

One note we might need to talk more about is if the Promise rejects.

@fregante
Copy link

fregante commented Sep 25, 2024

If returning promises in Chrome is a breaking change, accepting promises in sendResponse could be an alternative pattern:

chrome.runtime.onMessage.addListener((message, sender, sendResponse) => {
	sendResponse(asyncFunction())
});

I'd love to see this soon, especially with rejection propagation across contexts.

  • Firefox (I think) and webextension-polyfill serialize the error and throw it on the sendMessage call
  • Safari does not throw an error on sendMessage
  • Chrome does not accept promises yet

Rejection propagation currently requires manual error serialization and a setup like:

import {serializeError} from 'serialize-error';

chrome.runtime.onMessage.addListener((message, sender, sendResponse) => {
	asyncFunction().then(sendResponse, error => {
		sendResponse({$$error: serializeError(error)});
		throw error;
	});
	return true;
});
import {deserializeError} from 'serialize-error';

const response = await chrome.runtime.sendMessage('my message')

if (response.$$error) {
	// Preserves both stacks
	throw new Error('Response error', {
		cause: deserializeError(response.$$error);
	});
}
console.log(response)

@xeenon
Copy link
Collaborator

xeenon commented Sep 25, 2024

Safari does not throw an error on sendMessage

@fregante Can you expand on this Safari issue?

@fregante
Copy link

fregante commented Sep 25, 2024

I only partially tested Safari, but the repro should be:

// background.js
chrome.runtime.onMessage.addListener(async () => {
	throw new TypeError('catch me');
});
// contentscript.js
chrome.runtime.sendMessage('hi');
// => Promise<undefined>

I'd expect sendMessage to throw the received error, but instead it returns Promise<undefined>.

Mozilla's polyfill instead re-throws the error as expected:

https://github.com/mozilla/webextension-polyfill/blob/351ffad2e553e51add6f96050656c16ce87cc3ad/src/browser-polyfill.js#L485-L490

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
implemented: firefox Implemented in Firefox implemented: safari Implemented in Safari inconsistency Inconsistent behavior across browsers supportive: chrome Supportive from Chrome
Projects
None yet
Development

No branches or pull requests

6 participants