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

Multiple Event Message Issues in "webext-redux": "3.0.0-mv3.0" #303

Open
Lalg28 opened this issue Aug 26, 2024 · 3 comments · May be fixed by #304
Open

Multiple Event Message Issues in "webext-redux": "3.0.0-mv3.0" #303

Lalg28 opened this issue Aug 26, 2024 · 3 comments · May be fixed by #304
Assignees

Comments

@Lalg28
Copy link

Lalg28 commented Aug 26, 2024

Hello,

I’m experiencing several errors with the "webext-redux": "3.0.0-mv3.0" version, primarily related to event messages. The errors are persistent and occur under specific conditions. Below is an example:

Screenshot 2024-08-26 at 6 26 48 p m

Based on my research, I suspect these issues might be caused by the absence of the return true; statement in the message handler, as shown in the following code snippet:

if (request.message === 'webext-example') { doAction(); return true; }

This statement is crucial to ensure proper message handling, especially when dealing with asynchronous actions.

Any help or guidance on resolving this would be greatly appreciated!

@SidneyNemzer
Copy link
Collaborator

Thanks for the report! I missed this in my testing because I didn't check what happens when non-webext redux messages are received by the service worker. It looks like this code is the cause:

https://github.com/tshaddix/webext-redux/blob/master/src/listener.js#L10-L11

To fix this, the function above needs to conditionally return true, only for messages that will be handled by webext-redux. I'll think through how to implement this.

In the meantime you can ignore the error -- it shouldn't cause any issues in your app.

@SidneyNemzer SidneyNemzer self-assigned this Sep 4, 2024
@Lalg28
Copy link
Author

Lalg28 commented Sep 4, 2024

Thanks for your response, I'll take that in mind! 🤠

SidneyNemzer added a commit that referenced this issue Sep 20, 2024
fixes #303

The issue was that the deferred listener
abstraction would `return true` (telling the
browser to expect an async response) for every
message, even if the messages were not from the
library. In `wrapStore()`, those messages would be
ignored causing the browser to log an error:

    Error: A listener indicated an asynchronous response by returning true, but the message channel closed before a response was received

This is mostly harmless, but it's better to avoid
the error. In this commit,
`createDeferredListener()` now takes a `filter`
function that determines if the message will be
handled so that messages from outside the library
can be ignored.

This requires a breaking change: moving the
`channelName` argument to `createWrapStore()`.
This is required since the filter function needs
to know which channel to expect before
`wrapStore()` is called.
SidneyNemzer added a commit that referenced this issue Sep 20, 2024
fixes #303

The issue was that the deferred listener
abstraction would `return true` (telling the
browser to expect an async response) for every
message, even if the messages were not from the
library. In `wrapStore()`, those messages would be
ignored causing the browser to log an error:

    Error: A listener indicated an asynchronous response by returning true, but the message channel closed before a response was received

This is mostly harmless, but it's better to avoid
the error. In this commit,
`createDeferredListener()` now takes a `filter`
function that determines if the message will be
handled so that messages from outside the library
can be ignored.

This requires a breaking change: moving the
`channelName` argument to `createWrapStore()`.
This is required since the filter function needs
to know which channel to expect before
`wrapStore()` is called.
@SidneyNemzer SidneyNemzer linked a pull request Sep 20, 2024 that will close this issue
@SidneyNemzer
Copy link
Collaborator

Created a PR to address this if you're curious: #304

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

Successfully merging a pull request may close this issue.

5 participants
@SidneyNemzer @Lalg28 and others