Fix recursion depth error in _redact_exception_with_context#61776
Conversation
42caffe to
5326a05
Compare
shared/secrets_masker/src/airflow_shared/secrets_masker/secrets_masker.py
Outdated
Show resolved
Hide resolved
shared/secrets_masker/src/airflow_shared/secrets_masker/secrets_masker.py
Show resolved
Hide resolved
Fix recursion depth check in _redact_exception_with_context_or_cause. There are some obscure cases where exception might point to itself in cause/context - this PR protects against it. Changed name to include cause as well. Initially implemented as a fix to v2-11-test in apache#61254 - enhanced with the case of removal of too-deep exceptions rather than not-redacting it (and replacing it with sentinel exception explaining that reminder of the stack trace has been removed. Co-authored-by: Anton Nitochkin <nitochkin@google.com>
5326a05 to
cc6b24c
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the SecretsMasker exception redaction logic to safely handle circular __context__/__cause__ chains and to cap traversal using a recursion/visit limit, preventing RecursionError during log masking.
Changes:
- Replace
_redact_exception_with_contextwith_redact_exception_with_context_or_cause, adding cycle detection and a max-visit limit with a sentinel exception when truncating. - Update the log filter to call the new redaction helper.
- Add extensive unit tests covering context/cause chaining, cycles, branching, and depth-limit behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
shared/secrets_masker/src/airflow_shared/secrets_masker/secrets_masker.py |
Introduces cycle/limit-aware exception redaction and switches the filter to use it. |
shared/secrets_masker/tests/secrets_masker/test_secrets_masker.py |
Adds new tests for the updated exception redaction behavior across multiple chaining scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
shared/secrets_masker/src/airflow_shared/secrets_masker/secrets_masker.py
Show resolved
Hide resolved
shared/secrets_masker/tests/secrets_masker/test_secrets_masker.py
Outdated
Show resolved
Hide resolved
shared/secrets_masker/tests/secrets_masker/test_secrets_masker.py
Outdated
Show resolved
Hide resolved
jscheffl
left a comment
There was a problem hiding this comment.
Wanted to have AI used again. Not too bad as review. But approved anyway.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Not bad indeed. |
But I still see review rather as an opportunity to have human-human interaction. |
100% - I just started it as another pair of eyes :-D |
Backport failed to create: v2-11-test. View the failure log Run details
You can attempt to backport this manually by running: cherry_picker 1533ecf v2-11-testThis should apply the commit to the v2-11-test branch and leave the commit in conflict state marking After you have resolved the conflicts, you can continue the backport process by running: cherry_picker --continueIf you don't have cherry-picker installed, see the installation guide. |
…xt (#61776) * Fix recursion depth error in _redact_exception_with_context Fix recursion depth check in _redact_exception_with_context_or_cause. There are some obscure cases where exception might point to itself in cause/context - this PR protects against it. Changed name to include cause as well. Initially implemented as a fix to v2-11-test in #61254 - enhanced with the case of removal of too-deep exceptions rather than not-redacting it (and replacing it with sentinel exception explaining that reminder of the stack trace has been removed. Co-authored-by: Anton Nitochkin <nitochkin@google.com> * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- (cherry picked from commit 1533ecf) Co-authored-by: Jarek Potiuk <jarek@potiuk.com> Co-authored-by: Anton Nitochkin <nitochkin@google.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…xt (apache#61776) * Fix recursion depth error in _redact_exception_with_context Fix recursion depth check in _redact_exception_with_context_or_cause. There are some obscure cases where exception might point to itself in cause/context - this PR protects against it. Changed name to include cause as well. Initially implemented as a fix to v2-11-test in apache#61254 - enhanced with the case of removal of too-deep exceptions rather than not-redacting it (and replacing it with sentinel exception explaining that reminder of the stack trace has been removed. Co-authored-by: Anton Nitochkin <nitochkin@google.com> * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- (cherry picked from commit 1533ecf) Co-authored-by: Jarek Potiuk <jarek@potiuk.com> Co-authored-by: Anton Nitochkin <nitochkin@google.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…1776) * Fix recursion depth error in _redact_exception_with_context Fix recursion depth check in _redact_exception_with_context_or_cause. There are some obscure cases where exception might point to itself in cause/context - this PR protects against it. Changed name to include cause as well. Initially implemented as a fix to v2-11-test in apache#61254 - enhanced with the case of removal of too-deep exceptions rather than not-redacting it (and replacing it with sentinel exception explaining that reminder of the stack trace has been removed. Co-authored-by: Anton Nitochkin <nitochkin@google.com> * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Anton Nitochkin <nitochkin@google.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…1776) * Fix recursion depth error in _redact_exception_with_context Fix recursion depth check in _redact_exception_with_context_or_cause. There are some obscure cases where exception might point to itself in cause/context - this PR protects against it. Changed name to include cause as well. Initially implemented as a fix to v2-11-test in apache#61254 - enhanced with the case of removal of too-deep exceptions rather than not-redacting it (and replacing it with sentinel exception explaining that reminder of the stack trace has been removed. Co-authored-by: Anton Nitochkin <nitochkin@google.com> * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Anton Nitochkin <nitochkin@google.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…xt (#61776) * Fix recursion depth error in _redact_exception_with_context Fix recursion depth check in _redact_exception_with_context_or_cause. There are some obscure cases where exception might point to itself in cause/context - this PR protects against it. Changed name to include cause as well. Initially implemented as a fix to v2-11-test in #61254 - enhanced with the case of removal of too-deep exceptions rather than not-redacting it (and replacing it with sentinel exception explaining that reminder of the stack trace has been removed. Co-authored-by: Anton Nitochkin <nitochkin@google.com> * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- (cherry picked from commit 1533ecf) Co-authored-by: Jarek Potiuk <jarek@potiuk.com> Co-authored-by: Anton Nitochkin <nitochkin@google.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…xt (#61776) (#61795) * Fix recursion depth error in _redact_exception_with_context Fix recursion depth check in _redact_exception_with_context_or_cause. There are some obscure cases where exception might point to itself in cause/context - this PR protects against it. Changed name to include cause as well. Initially implemented as a fix to v2-11-test in #61254 - enhanced with the case of removal of too-deep exceptions rather than not-redacting it (and replacing it with sentinel exception explaining that reminder of the stack trace has been removed. * Apply suggestions from code review --------- (cherry picked from commit 1533ecf) Co-authored-by: Jarek Potiuk <jarek@potiuk.com> Co-authored-by: Anton Nitochkin <nitochkin@google.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Fix recursion depth check in _redact_exception_with_context_or_cause. There are some obscure cases where exception might point to itself in cause/context - this PR protects against it.
Changed name to include cause as well.
Initially implemented as a fix to v2-11-test in #61254 - enhanced with the case of removal of too-deep exceptions rather than not-redacting it (and replacing it with sentinel exception explaining that reminder of the stack trace has been removed.
Was generative AI tooling used to co-author this PR?
Generated-by: Copilot + Claude Sonnet 4.5 following the guidelines
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.