ADFA-2959 | Catch composition errors and handle preview crashes#1326
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughrender() centralizes preview error handling: it finds the composable via ComposableInvoker, wraps setContent in try/catch, uses in-composition state to show setup errors, and delegates reflective invocation and default-parameter assembly to ComposableInvoker with signature analysis by ComposeSignature. ChangesUnified error handling architecture
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableRenderer.kt`:
- Around line 56-59: In ComposableRenderer.kt replace the broad catch (e:
Throwable) in the preview composition block with catch (e: Exception) so that
fatal JVM Errors (e.g., OOM, StackOverflowError) are allowed to propagate; keep
the existing LOG.error(...) and showError(...) calls inside the catch (Exception
e) block to preserve logging and UI feedback, and do not swallow or rethrow
Errors anywhere in the composition handler.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4020f0c2-849b-4f4f-acaa-b24b77b5c59f
📒 Files selected for processing (1)
compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableRenderer.kt
87e787a to
e311852
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableRenderer.kt`:
- Around line 48-59: The try/catch around composeView.setContent won't catch
exceptions from later recompositions or effects; instead detect and capture
errors before entering the Compose tree and/or surface them via Compose state:
validate and catch reflective/initialization errors when resolving
clazz/composableMethod (the code that currently calls RenderComposable), store
the Throwable in a mutableState (e.g., previewErrorState), then call
composeView.setContent with a composable that reads that state and conditionally
renders either ErrorContent/showError UI or RenderComposable; also log via
LOG.error when you capture the Throwable so runtime exceptions are surfaced and
the preview UI shows the error instead of crashing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5da8d216-7cbc-4934-9e27-37d691e0f819
📒 Files selected for processing (1)
compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableRenderer.kt
e311852 to
a7c4439
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableRenderer.kt (1)
59-63: ⚡ Quick winPreserve the wrapper message in setup failures.
Unwrapping to
exception.cause ?: exceptiondrops the context added inRenderComposable(for example, which class failed to instantiate), so the UI and log often degrade to a raw reflection message. Prefer loggingexceptionand usingexception.messagebefore falling back to the cause.♻️ Suggested tweak
RenderComposable(clazz, composableMethod) { exception -> val cause = exception.cause ?: exception - LOG.error("Reflection error before composition", cause) - errorMessage.value = "Setup failed: ${cause.message ?: cause.javaClass.simpleName}" + LOG.error("Reflection error before composition", exception) + errorMessage.value = + "Setup failed: ${exception.message ?: cause.message ?: cause.javaClass.simpleName}" }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableRenderer.kt` around lines 59 - 63, The current handler unwraps the thrown exception to its cause, which loses context added by RenderComposable (e.g., which class failed); instead log the original exception object (LOG.error(..., exception)) and set errorMessage.value using exception.message first, then fall back to exception.cause?.message or exception.cause?.javaClass?.simpleName (and finally exception.javaClass.simpleName) so both logs and UI preserve the wrapper context provided by RenderComposable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableRenderer.kt`:
- Around line 113-120: The multi-slot invocation branch in RenderComposable
incorrectly assumes Composer and changed Int occupy the last two JVM parameters;
update the paramCount > 2 logic to inspect method.parameterTypes to locate the
Composer type and the Int "$changed" parameter index, build args[] with composer
placed at the discovered Composer index and 0 at the discovered changed-index,
and only call method.invoke(instance, *args) when both positions are found;
otherwise call onReflectionError with a clear message rejecting unsupported
signatures. Ensure you reference the existing variables/methods:
RenderComposable, method.parameterTypes, composer/currentComposer, args array
creation, method.invoke(...) and onReflectionError.
---
Nitpick comments:
In
`@compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableRenderer.kt`:
- Around line 59-63: The current handler unwraps the thrown exception to its
cause, which loses context added by RenderComposable (e.g., which class failed);
instead log the original exception object (LOG.error(..., exception)) and set
errorMessage.value using exception.message first, then fall back to
exception.cause?.message or exception.cause?.javaClass?.simpleName (and finally
exception.javaClass.simpleName) so both logs and UI preserve the wrapper context
provided by RenderComposable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c49c9759-7d3a-4c0b-95f3-4c4aba7613f1
📒 Files selected for processing (1)
compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableRenderer.kt
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposeSignature.kt (1)
8-12: 💤 Low valueConsider making
WithComposeradata classand dropping the unusedtotalParamsfield.
totalParamsis exactlytypes.size, so it's redundant. A small cleanup:♻️ Optional refactor
- class WithComposer( - val composerIndex: Int, - val totalParams: Int, - val types: Array<Class<*>> - ) : ComposeSignature() + data class WithComposer( + val composerIndex: Int, + val types: Array<Class<*>> + ) : ComposeSignature() { + val totalParams: Int get() = types.size + }
ComposableInvoker.invokeWithComposerwould continue to compile unchanged.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposeSignature.kt` around lines 8 - 12, Change the sealed subclass WithComposer to a data class and remove the redundant totalParams property (since totalParams == types.size); update its declaration so it only holds composerIndex and types (e.g., data class WithComposer(val composerIndex: Int, val types: Array<Class<*>>) : ComposeSignature()), and verify usages such as ComposableInvoker.invokeWithComposer compile without changes because callers should derive totalParams from types.size where needed.compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableInvoker.kt (2)
12-26: ⚡ Quick winSelection logic prefers the smallest mangled candidate, which may bypass the actual
@Composableentry point.When the exact-name lookup on line 15 fails, line 25 returns the candidate with the smallest
parameterCount. For a top-level@Composablethe Kotlin compiler emits the canonical method with the user's signature plus(Composer, Int [, Int])— that is the one you want to invoke. Mangled siblings ($lambda$0,$WhenMappings, capture wrappers, etc.) typically have fewer parameters than the canonical entry, sominByOrNull { parameterCount }will tend to pick the wrong one when the canonical name isn't found exactly.In practice the exact-name lookup on line 15 will hit for normal
@Composablefunctions, so this fallback may rarely be exercised — but when it is, it's likely to pick a helper instead of the composable. Consider preferring candidates whose parameter list contains aComposer(viaComposeSignature.analyze(...) is WithComposer) and using parameter count only as a tiebreaker.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableInvoker.kt` around lines 12 - 26, findComposableMethod's fallback picks the candidate with the smallest parameterCount which can select mangled helpers instead of the real `@Composable` entry; update the selection to prefer methods whose parameter signature includes a Composer (use ComposeSignature.analyze(method) and check for WithComposer) and only use parameterCount as a tiebreaker (e.g. filter or sort so WithComposer candidates rank above others), keeping the existing exclusion of "$default" and the mangled-name checks in the candidates collection.
20-23: 💤 Low valueRemove redundant
${functionName}$lambdaequality checkIn
compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableInvoker.kt(lines 20-23), themethod.name == "${functionName}\$lambda"branch is overly specific: Compose’s lambda memoization helper naming uses an ordinal suffix (e.g.lambda-<n>), so there won’t be a plain${functionName}$lambdamethod name. The existingstartsWith("$functionName\$")check already covers the real generated variants.♻️ Minor cleanup
val candidates = methods.filter { method -> !method.name.contains("\$default") && - (method.name.startsWith("$functionName\$") || method.name == "${functionName}\$lambda") + method.name.startsWith("$functionName\$") }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableInvoker.kt` around lines 20 - 23, The filter on methods in ComposableInvoker.kt is checking method.name == "${functionName}\$lambda" which is redundant and incorrect because generated lambda helpers use ordinal suffixes (e.g. "lambda-<n>") and are already matched by the existing startsWith("$functionName\$") condition; remove the equality branch from the predicate (the part comparing method.name to "${functionName}\$lambda") so the candidates list relies on the !method.name.contains("$default") and the startsWith("$functionName\$") checks only.compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableRenderer.kt (1)
58-62: 💤 Low valueState write during composition relies on the
ifbranch flipping to prevent re-entry.
onReflectionErroris invoked synchronously insideRenderComposable's composition. WritingerrorMessage.valuehere happens during a composition pass that already readerrorMessageon line 55. This works because once the value flips to non-null, the next composition takes theErrorContentbranch and never re-entersRenderComposable— so you don't loop. Just be aware:
- If the
if/elseon lines 55–63 is ever refactored such thatRenderComposableis also reachable after an error (e.g., a retry path), you'll get an infinite recomposition loop because the same reflection error is thrown each frame.- Compose's strict mode / snapshot tooling may flag in-composition writes; consider deferring the state write with
SideEffect { errorMessage.value = ... }if you start seeing snapshot warnings in CI. Not required today.No change requested.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableRenderer.kt` around lines 58 - 62, The code currently writes to errorMessage.value synchronously inside RenderComposable's composition via the onReflectionError lambda (the block passed to RenderComposable) which can cause in-composition state writes and potential infinite recomposition if RenderComposable remains reachable after an error; to harden this, change the error write to be deferred from the composition using a SideEffect (or otherwise schedule the state update off the read path) so the lambda logs the error with LOG.error(...) and then uses SideEffect { errorMessage.value = "Setup failed: ${cause.message ?: cause.javaClass.simpleName}" } to update state; target the RenderComposable invocation and the onReflectionError lambda where errorMessage is set to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableInvoker.kt`:
- Around line 50-69: invokeWithComposer constructs args with getDefaultValue for
pre-composer params and zeros for all trailing ints, which causes non-primitive
defaulted parameters to be passed as null while the `$default` mask bits remain
0 (meaning "use provided value") and leads to NPEs in composable bodies; update
invokeWithComposer to compute and set the `$default` mask bits for trailing-int
slots (using signature.composerIndex and signature.totalParams to locate the
trailing-int region) so that any user param whose defaulted value is null flips
the corresponding bit in the `$default` mask(s) (use per-parameter masks or
0xFFFFFFFF for 32-bit groups) instead of writing zeros for all trailing ints,
and ensure you distinguish `$changed` vs `$default` semantics when walking
trailing ints; keep error handling in RenderComposable/PreviewSetupException
unchanged but document this limitation if you choose a simpler mask strategy.
- Around line 28-48: The invokeSafely method currently lets reflection failures
escape normalization; wrap instance creation and all method invocation paths
through the existing executeInvocation path (or catch reflection exceptions and
rethrow as PreviewSetupException) so failures are consistently normalized for
RenderComposable to catch; specifically, move
clazz.getDeclaredConstructor().newInstance() into an executeInvocation call (or
catch
NoSuchMethodException/InstantiationException/IllegalAccessException/InvocationTargetException
and throw PreviewSetupException), call method.invoke(...) inside
executeInvocation (or have invokeWithComposer call executeInvocation for its
reflective call), and remove the now-unreachable instance == null guard; ensure
Unsupported signature still throws a PreviewSetupException rather than
IllegalArgumentException.
---
Nitpick comments:
In
`@compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableInvoker.kt`:
- Around line 12-26: findComposableMethod's fallback picks the candidate with
the smallest parameterCount which can select mangled helpers instead of the real
`@Composable` entry; update the selection to prefer methods whose parameter
signature includes a Composer (use ComposeSignature.analyze(method) and check
for WithComposer) and only use parameterCount as a tiebreaker (e.g. filter or
sort so WithComposer candidates rank above others), keeping the existing
exclusion of "$default" and the mangled-name checks in the candidates
collection.
- Around line 20-23: The filter on methods in ComposableInvoker.kt is checking
method.name == "${functionName}\$lambda" which is redundant and incorrect
because generated lambda helpers use ordinal suffixes (e.g. "lambda-<n>") and
are already matched by the existing startsWith("$functionName\$") condition;
remove the equality branch from the predicate (the part comparing method.name to
"${functionName}\$lambda") so the candidates list relies on the
!method.name.contains("$default") and the startsWith("$functionName\$") checks
only.
In
`@compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableRenderer.kt`:
- Around line 58-62: The code currently writes to errorMessage.value
synchronously inside RenderComposable's composition via the onReflectionError
lambda (the block passed to RenderComposable) which can cause in-composition
state writes and potential infinite recomposition if RenderComposable remains
reachable after an error; to harden this, change the error write to be deferred
from the composition using a SideEffect (or otherwise schedule the state update
off the read path) so the lambda logs the error with LOG.error(...) and then
uses SideEffect { errorMessage.value = "Setup failed: ${cause.message ?:
cause.javaClass.simpleName}" } to update state; target the RenderComposable
invocation and the onReflectionError lambda where errorMessage is set to locate
the change.
In
`@compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposeSignature.kt`:
- Around line 8-12: Change the sealed subclass WithComposer to a data class and
remove the redundant totalParams property (since totalParams == types.size);
update its declaration so it only holds composerIndex and types (e.g., data
class WithComposer(val composerIndex: Int, val types: Array<Class<*>>) :
ComposeSignature()), and verify usages such as
ComposableInvoker.invokeWithComposer compile without changes because callers
should derive totalParams from types.size where needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8ae1c34a-7ebb-4d7d-b5b5-4d9d153ed7a6
📒 Files selected for processing (3)
compose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableInvoker.ktcompose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposableRenderer.ktcompose-preview/src/main/java/com/itsaky/androidide/compose/preview/runtime/ComposeSignature.kt
30f0306 to
307b22a
Compare
…very - Modularize reflection logic into `ComposableInvoker` and `ComposeSignature` for better maintainability (SRP). - Implement dynamic mapping for Compose's synthetic `$changed` and `$default` trailing parameters to support complex signatures. - Use `Array.fill` to safely inject `-1` into `$default` mask slots, preventing NPEs on non-primitive default parameters. - Introduce `PreviewSetupException` to isolate and normalize reflection setup errors, routing them to the Compose state error UI. - Establish a safe outer try/catch boundary to survive genuine user-code crashes (e.g., RuntimeExceptions inside their Preview) and prevent fatal Compose Runtime "Start/end imbalance" corruptions.
307b22a to
9afb052
Compare
Description
This PR addresses an internal Jetpack Compose runtime crash (
ComposeRuntimeError: Compose Runtime internal error) caused by start/end api imbalance or unhandled exceptions during the preview composition process. The rendering phase is now enclosed within a robust try-catch structure to safely capture runtime execution failures, log them appropriately, and display a meaningful error state in the UI rather than crashing the execution context. Additionally, redundantrunCatchingwrappers on the composable method invocations have been unwrapped to avoid masking exceptions or creating structural runtime imbalances.Details
Before fix
Screen.Recording.2026-05-20.at.12.49.07.PM.mov
After fix
Screen.Recording.2026-05-20.at.12.51.31.PM.mov
Ticket
ADFA-2959
Observation
By converting the individual reflection invocation results from
runCatchingto standard top-level try-catch coverage at the composition initialization block level, we ensure the Jetpack Compose renderer accurately tracks state and balances node lifecycles without getting interrupted mid-execution.