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

Retry if registerMethods hasn't been called yet #77

Merged
merged 3 commits into from
Aug 19, 2022

Conversation

fregante
Copy link
Collaborator

Issue

The internal registerMethods call for __getTabData changes the behavior of the native sender because it adds the onMessage listener. This is not preventable.

The same behavior change also affected the custom sender which, instead of ignoring a context that didn't register any methods yet, it would receive the error "No METHOD registered here".

Messaging a target

Native behavior

webext-messenger extension of behavior

  • if the messenger isn't loaded, the sender retries
  • 🆕 if the messenger is loaded but registerMethods isn't called, the sender retries
  • if the messenger is loaded and registerMethods is called, but the specific METHOD isn't registered, the sender throws "No METHOD found"

Related

Future improvements

  • Clarify what "No METHOD registered in the receiving end" means, possibly replacing the message with "The receiving end did not handle the METHOD request" or something like that.

Comment on lines +103 to +104
// `registerMethods` not yet loaded
String(error.message).startsWith("No handlers registered in ")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line will make the sender retry when it receives this new error.

@fregante fregante merged commit c46e737 into main Aug 19, 2022
@fregante fregante deleted the wait-for-registerMethods branch August 19, 2022 18:50
@fregante
Copy link
Collaborator Author

This essentially fixes a regression caused by

😅

The Messenger however continues to work under the assumption that if a target receives a message and the specific METHOD isn't defined, it's an error. Changing this assumption might not be straightforward and is not needed at this stage.

export const handlers = new Map<string, Method>();

export function didUserRegisterMethods(): boolean {
return handlers.size > privateMethods.length;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can reimplement this better actually and completely drop the private registerMethods call from the content scripts, for example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
1 participant