ADFA-4059: capture debug info when SourceFileIndexer fails#1335
Conversation
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
|
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 (2)
📝 Walkthrough
Risks and best-practice considerations:
WalkthroughAdds Sentry dependency and instruments the Kotlin LSP: build completion, compilation environment refresh, symbol index refresh, and source-file analysis now emit Sentry breadcrumbs or report exceptions; module scope creation/invalidation includes breadcrumbs; moduleDescription overrides added; module virtual-file searches no longer share a visited set. ChangesSentry instrumentation and module lifecycle
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
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 docstrings
🧪 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
`@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/SourceFileIndexer.kt`:
- Around line 123-134: The current Sentry.captureException block is adding full
attachment.encodedBytes into extras (inside the err is
KotlinExceptionWithAttachments branch) which can exceed Sentry limits and leak
data; modify the code that iterates err.attachments in the Sentry.scope.apply
lambda to stop calling setExtra with attachment.encodedBytes and instead record
only safe metadata (e.g., attachment.path, encoded byte length, and a
hash/checksum) and attach the actual binary via Sentry’s attachment mechanism
(or omit the payload entirely) so the extras remain small and non-sensitive;
look for the err.attachments.forEachIndexed loop and replace encodedBytes
setExtra with metadata fields and/or use Sentry attachment APIs.
🪄 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: f9ccd07e-f7db-437b-b924-dbec93cdc793
📒 Files selected for processing (8)
lsp/kotlin/build.gradle.ktslsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinLanguageServer.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/CompilationEnvironment.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/KtSymbolIndex.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/SourceFileIndexer.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AbstractKtModule.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/services/ProjectStructureProvider.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/KotlinCompletions.kt
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtSourceModule.kt (1)
83-85:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftVerify the delegation target for
moduleDescription.Same concern as in
KtLibraryModule.kt: this override delegates tosuper<KaSourceModule>.moduleDescription(the interface) instead ofsuper<AbstractKtModule>.moduleDescription. If the customAbstractKtModuleimplementation contains Sentry instrumentation with debug context (module id, scope hashes, dependency ids), this delegation bypasses that information.Please verify the delegation target is correct for capturing debug info as intended by the PR objective.
🤖 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 `@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtSourceModule.kt` around lines 83 - 85, The override of moduleDescription in KtSourceModule currently delegates to super<KaSourceModule>.moduleDescription (the interface) which may bypass AbstractKtModule's Sentry/debug instrumentation; change the delegation target to super<AbstractKtModule>.moduleDescription so the AbstractKtModule implementation (and its debug context like module id, scope hashes, dependency ids) is executed—adjust the override in KtSourceModule.kt (keeping the `@OptIn` annotation) to call super<AbstractKtModule>.moduleDescription instead of super<KaSourceModule>.moduleDescription.
🤖 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
`@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtLibraryModule.kt`:
- Around line 117-119: KtLibraryModule.moduleDescription currently delegates to
super<KaLibraryModule>.moduleDescription which can lose the detailed debug
context built by AbstractKtModule; change the delegation to return
super<AbstractKtModule>.moduleDescription (or otherwise call the
AbstractKtModule implementation) so the id, _baseSearchScope/_contentScope hash
codes and dependency ids are preserved for Sentry/debugging; update the override
in KtLibraryModule to reference AbstractKtModule's moduleDescription
implementation instead of KaLibraryModule's.
---
Duplicate comments:
In
`@lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtSourceModule.kt`:
- Around line 83-85: The override of moduleDescription in KtSourceModule
currently delegates to super<KaSourceModule>.moduleDescription (the interface)
which may bypass AbstractKtModule's Sentry/debug instrumentation; change the
delegation target to super<AbstractKtModule>.moduleDescription so the
AbstractKtModule implementation (and its debug context like module id, scope
hashes, dependency ids) is executed—adjust the override in KtSourceModule.kt
(keeping the `@OptIn` annotation) to call
super<AbstractKtModule>.moduleDescription instead of
super<KaSourceModule>.moduleDescription.
🪄 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: eda85cc5-ea20-4705-ab88-c347646ddb28
📒 Files selected for processing (3)
lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/SourceFileIndexer.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtLibraryModule.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/KtSourceModule.kt
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
See ADFA-4059 for more details.