Skip to content
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 exc_type being exception instance rather than type #8257

Merged
merged 1 commit into from
May 23, 2023
Merged

Fix exc_type being exception instance rather than type #8257

merged 1 commit into from
May 23, 2023

Conversation

Mapiarz
Copy link

@Mapiarz Mapiarz commented May 22, 2023

This PR #8149 introduced a regression which will cause some log handlers to crash. In my particular case, it is structlog with ConsoleRenderer (which uses rich under the hood). The reason for that I believe is 3ce5b85#diff-fb194522c454315cd87ccb1cf4a2a53aafa49124c85abc01dcca4310cf19d6d3R225 which passes the exception instance rather than the exception type. Some handlers will inspect and expect there to be a type and that will lead to crashes.

@auvipy auvipy self-requested a review May 23, 2023 14:16
@auvipy
Copy link
Member

auvipy commented May 23, 2023

@youtux can you verify, please?

@codecov
Copy link

codecov bot commented May 23, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (e7b47a6) 87.07% compared to head (60e0287) 87.07%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8257   +/-   ##
=======================================
  Coverage   87.07%   87.07%           
=======================================
  Files         148      148           
  Lines       18467    18467           
  Branches     2524     2524           
=======================================
  Hits        16081    16081           
  Misses       2107     2107           
  Partials      279      279           
Flag Coverage Δ
unittests 87.04% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
celery/app/trace.py 98.34% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@auvipy auvipy added this to the 5.3 milestone May 23, 2023
@youtux
Copy link
Contributor

youtux commented May 23, 2023

Ah yes, that makes sense!

@auvipy auvipy merged commit 1eee438 into celery:main May 23, 2023
25 checks passed
@Mapiarz Mapiarz deleted the fix-exception-type-trace branch May 24, 2023 11:12
@jbergler jbergler mentioned this pull request May 29, 2023
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants