-
Notifications
You must be signed in to change notification settings - Fork 95
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
Use original stacktrace on errors that happen in Cachex.fetch! fallbacks #252
Comments
@whitfin can I please get feedback on this? If this is something that would make sense for the project, I will fix the remaining issues and submit a PR |
@rinpatch just wanted to say thanks for this commit, providing good inspiration to me to better handle errors inside our computations functions (e.g. using Ecto, which can raise on its own). |
@rinpatch this is awesome; sorry to get back to you so late. If you have anything for this, feel free to throw it in a PR and we can get it included. I'll hold off on cutting v3.4.0 for a while so we can try get this in (no pressure). |
I'm gonna land this in v3.5.0 based on the concept in the linked commits. Looks good enough, just a couple of smaller tweaks, then it should be in v3.5.0 when shipped! 🎉 |
I'm wondering something... would it be possible to include the whole rescued exception in ExecutionError? this way we could re-raise it in the caller if needed, and we would keep all its fields |
@pdelacroix yes, I'm considering this. It's a bit wasteful, I would rather just remove |
Sometimes the fallback function logic can be quite complex and call a lot of functions, when these functions raise unexpectedly raise errors Cachex just returns
Cachex.ExecutionError
with the error message copied from the exception. I've been in multiple situations when the error message is not enough to identify where the issue happens and you have to check the all executed code manually or comment out the fetch and try to reproduce the issue without caching.It would be much faster do debug some issues if original stacktrace was available (currently the stacktrace in exceptions cachex throws just ends at
Cachex.unwrap_unsafe
). I wrote a proof of concept patch that does just that f03beed , here is how it looks:If you think this feature makes sense upstream, I will finish the TODOs and submit a PR.
The text was updated successfully, but these errors were encountered: