-
Notifications
You must be signed in to change notification settings - Fork 569
fix: Exceptions include detail property for their value #2193
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
Conversation
Exceptions with `detail` but no `message` add that property to the exception value. Fixes getsentryGH-2192
sentrivana
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nicolassanmar Thanks a ton for this PR! The change looks good, left a small comment. Could you please also add a testcase or two for this?
Sure, been busy this week, but I will update this when I can |
@sentrivana I have added test to this PR. For some reason, the new commit doesn't show up here and shows on a new branch. |
|
Thanks @goharShoukat! I'll take a look sometime this week. |
|
@nicolassanmar @goharShoukat Thank you both! |
|
Been following the changes you made Ivana. I had a question. The tests I wrote worked in Python 3.10. Is the attempt here to make the code backward compatible too? This is my first every open-source contribution, albeit a very small one, I am still quite proud to make it. Looking forward to contributing in the future. |
|
@goharShoukat It's a great first contribution! Congrats! :) The change I made to the test had to do with the change I made to the behavior of the Hope that makes sense. Looking forward to more contributions from you in the future! |
Exceptions with
detailbut nomessageadd that property to the exception value.Fixes #2192