Repository navigation
fix: Narrow Throwable catches where LinkageError is expected (JAVA-552) - #6562
Draft
runningcode wants to merge 2 commits into
Draft
runningcode wants to merge 2 commits into
runningcode wants to merge 2 commits into
Conversation
ExceptionUtils.rethrowIfFatal rethrows LinkageError, but at these sites a LinkageError is the expected failure: a native library that failed to load, an optional dependency that is missing, or a Compose or androidx version that doesn't match. Catch it explicitly first, and narrow the rest to the types the guarded code throws. Where the code calls app views, user integrations or JNI, keep the broad catch and call rethrowIfFatal after the LinkageError arm. Also stop QueueFile from deleting its file on an OutOfMemoryError, stop Installation from wrapping one in a RuntimeException, and stop the Ktor client integration from swallowing coroutine cancellation while reading the response body. Refs JAVA-552 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
📲 Install BuildsAndroid
|
5 of 9 tasks
This branch has not been deployed
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.
📜 Description
ExceptionUtils.rethrowIfFatalrethrowsLinkageError. At the 28catch (Throwable)sites inthis PR, a
LinkageErroris the expected failure, not a fatal one. Each one comes from a nativelibrary that failed to load, a missing optional dependency, or a mismatched Compose or androidx
version. This PR makes those sites safe before
rethrowIfFatalis added anywhere else:LoadClass, the Spotlight probe inSentryOptions, andProfilingServiceLoaderNdkIntegrationand theSentryNdknative library loadAppState, theAbstractMethodErrorcase from fix: AbstractMethodError when getting Lifecycle #2228Windows.kt,Nodes.ktandComposeViewHierarchyNode, and inSentryComposeHelperLinkageErrorfirst, then callrethrowIfFatalin thecatch (Throwable)and say in a commentwhy it's broad:
Integration.registerinSentry.initViewHierarchyEventProcessorandSentryWindowCallbackDebugImagesLoaderSentryContextStorageProviderNodes.ktandComposeViewHierarchyNodeUncaughtExceptionHandlerIntegration: stays broad withoutrethrowIfFatal. A rethrow therewould skip the chained default handler. Only a comment is added.
It also fixes three bugs found during the audit:
QueueFiledeleted its file when it hit anOutOfMemoryError.Installationwrapped anOutOfMemoryErrorin aRuntimeException.for a failed-request event.
💡 Motivation and Context
This is the first part of the
catch (Throwable)audit. About 176 other sites are boundariesthat should call
rethrowIfFatal, and about 103 should be narrowed. Doing that mechanically beforethis PR lands would crash apps that work today, for example:
Sentry.initregisters an integration whose optional dependency is missing💚 How did you test it?
SentryTest: an integration that throws aLinkageErroror aRuntimeExceptionis skippedand the next one still registers; an
OutOfMemoryErrorpropagates out ofSentry.init.LoadClassTest: a class whose static initializer throws returnsnull.sentry,sentry-android-core,sentry-android-ndk,sentry-android-replay,sentry-compose,sentry-opentelemetry-bootstrap,sentry-ktor-clientandsentry-async-profiler, plus detekt.📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
RejectedExecutionExceptionaroundexecutor.submit, file I/O, and parsing.rethrowIfFataland a comment to the boundary sites, one PR per module group.🤖 Generated with Claude Code