-
Notifications
You must be signed in to change notification settings - Fork 556
feat: Turn the unraisable exception integration on by default #4749
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
base: potel-base
Are you sure you want to change the base?
Conversation
❌ 1335 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
_DEFAULT_INTEGRATIONS.append( | ||
"sentry_sdk.integrations.unraisablehook.UnraisablehookIntegration" | ||
) |
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.
Potential bug: Adding a non-existent integration causes an unhandled `ModuleNotFoundError` in `iter_default_integrations`, crashing applications during `sentry_sdk.init()`.
-
Description: The
iter_default_integrations
function attempts to importsentry_sdk.integrations.unraisablehook.UnraisablehookIntegration
, which does not exist in the codebase. This action triggers aModuleNotFoundError
. Thetry...except
block within the function is not configured to catch this specific exception, as it only handlesDidNotEnable
andSyntaxError
. As a result, the unhandledModuleNotFoundError
will propagate up the call stack during SDK initialization (sentry_sdk.init()
), causing any application using the SDK with default settings to crash on startup. -
Suggested fix: Add
ModuleNotFoundError
to theexcept
clause in theiter_default_integrations
function. This will ensure that attempting to load a non-existent integration is gracefully handled by logging a debug message and continuing, which matches the intended behavior for other integration loading failures.
severity: 0.9, confidence: 0.98
Did we get this right? 👍 / 👎 to inform future reviews.
See #4733 (comment)