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

Avoid stack level to deep when trying to serialize the exception obje… #308

Merged

Conversation

mediazard
Copy link
Contributor

Avoid stack level to deep when trying to serialize the exception object in maxretry handler

@@ -1,3 +1,3 @@
module Sneakers
VERSION = "2.5.0"
VERSION = "2.5.1"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Never ever bump version in a pull request. It's up to the maintainers to decide when and why it should be done.

@michaelklishin
Copy link
Collaborator

@gabrieljoelc @jondot WDYT? This loses exception stack trace information but perhaps in this particular case that's not unreasonable?

@mediazard mediazard force-pushed the exception_serialization_in_maxretry_handler branch from a0651e9 to 93dca2b Compare August 2, 2017 15:42
@mediazard
Copy link
Contributor Author

Can you explain better what do you mean ?
https://github.com/reevoo/sneakers/blob/93dca2b31f6116107d00136d055304fa791bb882/lib/sneakers/handlers/maxretry.rb#L147
The reason is still exception object , so why do you think it's not possible to get backtrace info ?
We converted to string just when we assigned to error of data object.

@michaelklishin
Copy link
Collaborator

OK, if backtrace is included as a separate field then it's of no concern. Thanks.

@robinbortlik
Copy link

@michaelklishin @gabrieljoelc @jondot do we know when this fix will be merged?
Thank you :)

@gabrieljoelc gabrieljoelc merged commit 3dd8c61 into jondot:master Aug 24, 2017
@gabrieljoelc
Copy link
Collaborator

I'll bump the gem later

@gabrieljoelc gabrieljoelc mentioned this pull request Aug 24, 2017
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.

4 participants