Fix two crashes from unguarded calls into the optional Rich dependency - #837
Open
pranjalm37 wants to merge 2 commits into
Open
Fix two crashes from unguarded calls into the optional Rich dependency#837pranjalm37 wants to merge 2 commits into
pranjalm37 wants to merge 2 commits into
Conversation
…ndency - tracebacks.to_repr(): rich.pretty.traverse() was called with no error handling, unlike the non-Rich fallback right below it which explicitly catches everything. Any exception raised during Rich's own object introspection (e.g. from unusual attribute access) would propagate and break exception logging entirely, at exactly the moment it's needed most. Now falls back to the manual repr algorithm on failure. (hynek#655) - dev.RichTracebackFormatter: always passed locals_hide_dunder and locals_hide_sunder to Traceback.from_exception(), but those arguments were only added in Rich 13.1.0. On older Rich versions this raised TypeError: unexpected keyword argument, crashing the console renderer. Now inspects Traceback.from_exception()'s signature once and only passes the kwargs the installed Rich version actually supports, mirroring the existing hasattr(tb, "code_width") compatibility pattern already used a few lines below for the same reason. (hynek#576) Added regression tests for both and a changelog entry.
Without an explicit annotation, mypy inferred a narrow union type from the dict literal, which it then rejected when unpacked into Traceback.from_exception()'s specifically-typed parameters. CI caught this.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #655 and #576. Both are crashes caused by calling into the optional
richdependency without the same defensive handling structlog applieselsewhere.
#655 —
to_repr()propagates exceptions from Rich's introspectionrich.pretty.traverse(...).render()was called with no error handling,unlike the non-Rich fallback path right below it, which explicitly catches
everything ("Also catch all errors, similarly to
safe_str()"). If Rich'sown object introspection raises (e.g. on an object with unusual attribute
access, as in the anyio-internal object from the report), the exception
propagates out of
to_repr()and breaks exception logging entirely — atexactly the moment you need it most.
Fix: wrap the Rich call in
try/exceptand fall back to the manual repralgorithm on failure, so
to_repr()actually honors its own docstring("Get repr string for an object, but catch errors").
#576 —
RichTracebackFormattercrashes on older Rich versionsRichTracebackFormatter.__call__()always passeslocals_hide_dunderandlocals_hide_sundertoTraceback.from_exception(). Those arguments wereonly added in Rich 13.1.0, so on older Rich (e.g. 13.0.1, as in the report)
this raises
TypeError: Traceback.from_exception() got an unexpected keyword argument 'locals_hide_dunder', crashing the console rendereroutright.
Fix: inspect
Traceback.from_exception's signature once (module-level,next to the existing Rich import) and only pass the kwargs the installed
Rich version actually supports — mirroring the
hasattr(tb, "code_width")compatibility check already a few lines below, which exists for the same
reason (forward compat with
code_widthin that case, backward compatwith these two here).
Test plan
test_to_repr_rich_error— monkeypatchesrich.pretty.traversetoraise and asserts
to_repr()still returns a correct plain reprinstead of propagating.
test_old_rich_missing_locals_hide_params— monkeypatches thesupported-params set to simulate an old Rich install and asserts
locals_hide_dunder/locals_hide_sunderare omitted from the call.rich==13.0.1locallyand confirmed the
TypeErrorbefore the fix / clean success after.pytest tests/→ 902 passed, 21 skipped (up from 900passed pre-change — the 2 new tests).
ruff check/ruff formatclean on all changed files.## [Unreleased].