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

Passing unserializable values into action logger causes system to hang #1839

Closed
Billy- opened this issue Sep 13, 2017 · 11 comments
Closed

Passing unserializable values into action logger causes system to hang #1839

Billy- opened this issue Sep 13, 2017 · 11 comments

Comments

@Billy-
Copy link

Billy- commented Sep 13, 2017

Title pretty much says it all. Kinda related to #132, you just need to check if the object is serializable rather than only checking if it's a SyntheticEvent type

@Billy-
Copy link
Author

Billy- commented Sep 13, 2017

Of course, the obvious workaround is to pull out / modify the values:

const logOnChange = action('onChange')
...
    onChange={(foo, unserializable) => logOnChange(foo)}
...

@danielduan
Copy link
Member

You're more than welcome to open a PR to fix the issue

@Billy-
Copy link
Author

Billy- commented Sep 18, 2017

If it were as simple as wrapping serialize in a try {} catch then I would. However as mentioned it causes the system to hang -- and no errors are output at the end of it. So I'm not really sure where to go from there.

@dangreenisrael
Copy link
Member

@billy - Could you post detailed instructions on how to replicate this issue.

@danielduan
Copy link
Member

danielduan commented Sep 19, 2017

I think it has to do with how our addon channels serialize the communication so it's probably not just the addon-actions.

something like this should cause it to hang when it should be dropping the object key:

const a = {
  b: this
}

related to #778

@Billy-
Copy link
Author

Billy- commented Sep 19, 2017

@dangreenisrael I have come across this issue while using react-flatpickr. Just throw a quick story together for it and add an action hander to the onChange:

...
onChange={action('onChange')}
...

This causes the window to freeze up for around 20-30 seconds and then resumes and nothing happens

@danielduan
Copy link
Member

@Billy- might be related to this? haoxins/react-flatpickr#43

@Billy-
Copy link
Author

Billy- commented Sep 20, 2017

I don't think so. As mentioned above, this issue can be worked around by using an arrow function and pulling out only the variables you want. It is only if you pass the action logger straight into the onChange that it happens - because the second parameter is unserializable

@dangreenisrael
Copy link
Member

@Billy- Could you post a new repo reproducing this problem without the use of 'react-flatpikr'?

@Billy-
Copy link
Author

Billy- commented Sep 21, 2017

I don't really know the exact cause of the bug, so I'd have to put some time into actually trying to track that down. Unfortunately I don't really have the time to contribute that. Sincerest apologies!

@Hypnosphi
Copy link
Member

Fixed in 3.3.0-alpha.4

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

No branches or pull requests

4 participants