-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix django unicode error #2217
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
Fix django unicode error #2217
Conversation
debug_toolbar/panels/settings.py
Outdated
def catch_force_errors(force_function, value): | ||
try: | ||
return force_function(value) | ||
except DjangoUnicodeDecodeError: | ||
return "Debug toolbar was unable to parse value" |
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.
There are a few places we use force_str
and I suspect we may want to swap all of them out with this new function. Perhaps we should create a sanitize.py
file and put this in there with a doc string explaining the interface and what it does. Then use that everywhere we've been using force_str
. I'd consider keeping the name as force_str
too, but that's just my preference. We'd want to have an explicit test covering the functionality of this new function too.
There are some other sanitizing functions in utils.py
that can be moved over too, though that should be a separate commit. Or I can do that.
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.
Thank you @jmgutu for putting this together quickly. I think there are a few changes we should make to improve the toolbar as a whole and to make the developer experience a bit better. Please let me know if you think differently.
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.
Looks good, thank you for finding the clean-up opportunity in tracking.py
!
for more information, see https://pre-commit.ci
Co-authored-by: Tim Schilling <[email protected]>
Co-authored-by: Tim Schilling <[email protected]>
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
86262ee
to
d85033e
Compare
Description
The force serialize function in [django-debug-toolbar/debug_toolbar/panels/settings.py] was throwing errors for values that are not serializable. The proposed solution returns the serialized value if the value is serializable or returns 'Debug toolbar was unable to parse value'
Fixes #2172
Checklist:
docs/changes.rst
.