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

Pass complete serialized errors between background and proxy store #262

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Conversation

jlalmes
Copy link

@jlalmes jlalmes commented Oct 27, 2020

I've been facing the issue where webext-redux only allows you to error.message back to my content scripts in the case of an error in an aliased action. However my error objects contain other information, for example:

Error {
  message: 'this is a message',
  status: 400,
  code: 'CODE-001',
  result: { ... },
  stack: { ... },
}

This pull request will serialize all errors by default, the full error is then reconciled and rejected to the content-script that called the original dispatch function. You can test this PR with this npm module webext-redux-w-errors@latest.

First-time contributor, open to any feedback! 🙌

@jlalmes
Copy link
Author

jlalmes commented Nov 12, 2020

@tshaddix @vhmth - Hi guys seeking some guidance here. I am dispatching async actions with aliases and am therefore needing to return the full payload of a rejected error back to the content script via the proxy store. Are you happy to merge and publish this?

Copy link
Collaborator

@vhmth vhmth left a comment

Choose a reason for hiding this comment

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

Thank you for contributing here! 🙏 Just a heads up the reason @tshaddix and I tend to be slow to reply is because our jobs are both extremely demanding. I'm really sorry about the lag here!

This seems very reasonable, and I'm very appreciative of this for the Loom extension itself. Just looked through the code to see where else errors are dispatched, but it seems this is the main throughpoint.

cc @tshaddix any thoughts here?

Before we land, could you check out the tests folder and add an appropriate test there that checks that an error can be dispatched properly and received in an expected manner?

https://github.com/tshaddix/webext-redux/tree/master/test

@@ -1,6 +1,6 @@
{
"name": "webext-redux",
"version": "2.1.7",
"name": "webext-redux-w-errors",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we change back the name here and in package.json?

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 this pull request may close these issues.

2 participants