ADFA-4755: stop reporting benign stale-classpath failures from Kotlin LSP indexer

[hal-eisen-adfa]

Summary

Fixes ADFA-4755 / GlitchTip CODEONTHEGO-D (KotlinIllegalArgumentExceptionWithAttachments: Error while resolving ... from RAW_FIR to TYPES).

This issue is not a crash — it's our own telemetry. SourceFileIndexer.analyzeDeclaration already wraps per-declaration analysis in runCatching { ... }.onFailure { Sentry.captureException(err) }. The observed stack originates in analyzeProperty → KaPropertySymbol.returnType, inside that runCatching, so the exception is caught, the symbol skipped, and indexing continues (userCount: 0). The Sentry.captureException call — which forwards the FIR KotlinExceptionWithAttachments attachments — is exactly what emits the GlitchTip event.

Root cause is benign and transient: FIR resolution tried to read a dependency jar (.../.gradle/caches/8.14.3/transforms/.../lifecycle-viewmodel-2.5.1/jars/classes.jar) that no longer existed — a Gradle transform-cache entry evicted or rebuilt while indexing was in flight. The next index pass, after the build settles, succeeds. Reporting it as an error is noise.

Changes

• Throwable.isMissingClasspathFile() — walks the cause chain (bounded against a self-referential cause) for NoSuchFileException / FileNotFoundException.
• analyzeDeclaration — skip Sentry.captureException for these; debug-log instead. Kills the GlitchTip noise.
• indexSourceFile — wrap the analysis block in the module's standard degrade guard (rethrow CancellationException; log + report + skip otherwise). Defense in depth: a setup-time escape — outside the per-declaration runCatching — would otherwise propagate into KtSymbolIndex.scope, which has no CoroutineExceptionHandler, and crash the app. This is the exact hazard OrganizeImportsAction already documents.

Testing

• :lsp:kotlin:testV8DebugUnitTest --tests "...SourceFileIndexerTest" — 8/8 pass (5 new isMissingClasspathFile predicate tests: direct, wrapped, deeply nested, FileNotFoundException, and negative cases; plus the 3 pre-existing indexer tests).
• :app:assembleV8Debug — builds.

The live condition (a cache jar vanishing mid-index) isn't deterministically reproducible on device; correctness rests on the predicate tests plus the guard matching the module's established degrade pattern (KotlinDiagnosticProvider, KotlinCompletions).

Reviewer note

Most of the file diff is a mandatory Spotless full-file reformat the origin/stage ratchet triggers once the file is touched (grandfathered param/expression wrapping, trailing commas). Review the behavioral change with git diff -w.

Out of scope

Making the Kotlin analysis classpath resilient to transform-cache churn (re-resolving vanished jars) is a much larger change; this PR silences the benign telemetry and removes the crash risk.

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}

[coderabbitai]

Warning

Review limit reached

@hal-eisen-adfa, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 34 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 766dd254-b76d-4466-a04e-b8f8efaa6688

📥 Commits

Reviewing files that changed from the base of the PR and between 6ee65cc and 544f816.

📒 Files selected for processing (1)

• lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/SourceFileIndexer.kt

📝 Walkthrough

Walkthrough

Source indexing now detects missing classpath files, skips transient failures, reports other analysis errors, preserves cancellation, and adds tests for nested exception handling. Symbol-analysis construction is reformatted without changing field mappings.

Changes

Classpath-aware source indexing

Layer / File(s)	Summary
Missing classpath failure handling lsp/kotlin/src/main/.../SourceFileIndexer.kt	Adds cause-chain detection, cancellation propagation, transient-failure skipping, logging, and Sentry reporting for other failures.
Symbol analysis construction lsp/kotlin/src/main/.../SourceFileIndexer.kt	Reformats symbol, naming, visibility, metadata, and type construction while preserving existing mappings.
Indexer and helper tests lsp/kotlin/src/test/.../SourceFileIndexerTest.kt	Preserves indexing assertions and tests direct, wrapped, nested, and unrelated exception cases.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SourceFileIndexer
  participant AnalysisSession
  participant Logger
  participant Sentry
  SourceFileIndexer->>AnalysisSession: extract symbols
  AnalysisSession-->>SourceFileIndexer: analysis failure
  SourceFileIndexer->>SourceFileIndexer: inspect cause chain
  SourceFileIndexer->>Logger: debug-log missing classpath failure
  SourceFileIndexer->>Sentry: report other analysis failure

Loading

Possibly related PRs

• appdevforall/CodeOnTheGo#856: Classifies missing-file failures and skips Sentry reporting for expected missing-file conditions.

Suggested reviewers: itsaky-adfa

Poem

I’m a rabbit guarding jars in the night,
Missing classpaths vanish from sight.
Errors hop to Sentry, cancellations flee,
Symbols stay tidy as tidy can be.
Tests nibble every exception chain—
Thump, thump, indexing works again!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: suppressing benign stale-classpath failures from the Kotlin LSP indexer.
Description check	✅ Passed	The description is detailed and directly matches the code changes and testing described in the summary.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ADFA-4755-KotlinIllegalArgumentExceptionWithAttachments

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 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 187-191: Update the onFailure handler around the runCatching call
in SourceFileIndexer to detect CancellationException and immediately rethrow it
before handling missing classpath files or reporting failures. Preserve the
existing isMissingClasspathFile handling for all other errors.
- Around line 158-166: In the exception handler surrounding the indexing
operation in SourceFileIndexer, replace the broad Throwable catch with handling
for the expected recoverable exception types, such as RuntimeException from FIR
wrappers and IOException for missing classpath files. Preserve the existing
isMissingClasspathFile branching, logging, Sentry capture, and return behavior,
while allowing fatal JVM errors such as OutOfMemoryError and StackOverflowError
to propagate.

🪄 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: 1cf7dd0d-230d-4bbd-9861-6eca7e3be89e

📥 Commits

Reviewing files that changed from the base of the PR and between 8077407 and b6f005d.

📒 Files selected for processing (2)

• lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/SourceFileIndexer.kt
• lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/compiler/index/SourceFileIndexerTest.kt

[hal-eisen-adfa]

Code review (Claude Code, xhigh)

Scope: SourceFileIndexer.kt + SourceFileIndexerTest.kt. ~90% of the diff is mechanical ktlint reformatting with no behavior change.

Verdict: The core fix is sound — it catches the real crash origin (analyze() in analyzeMaybeDangling), rethrows cancellation first, and sits at the right layer so both the direct and debounced worker paths are protected. ProcessCanceledException extends java.util.concurrent.CancellationException so top-level cancellation is handled correctly; the kotlinx.coroutines.CancellationException typealias resolves to the same class abortIfCancelled() throws; no compile / non-ASCII / tab issues. The findings below concern the degrade path's after-state, benign-vs-real classification, fix depth, and test coverage. None block; #1-#3 are worth addressing.

Correctness

1. Per-declaration missing-jar skip commits a partial index marked isIndexed=true, then it's skipped forever — silently. SourceFileIndexer.kt:194 (effect at :172-173)
FIR resolves types lazily inside analyzeDeclaration (dcl.symbol, returnType, superTypes read the jar), so a vanished jar usually throws per declaration, not at session setup. analyzeDeclaration.onFailure now matches isMissingClasspathFile(), logs at debug, and return@onFailure -> getOrNull() -> null; the visitor continues, analyzeMaybeDangling returns a partial symbol list with no exception, and indexSourceFile runs insertAll(partial) + upsert(newFile.copy(isIndexed=true)) with current stamps. On the next pass the file is unchanged, so shouldBeSkipped returns true and it's never re-indexed until the user edits it. The partial-index outcome pre-dates this PR, but the PR removes the former Sentry.captureException signal for these failures, so the degradation is now invisible — and it defeats the fix's stated "a later index pass succeeds."

2. Wrapped cancellation is misrouted to Sentry-as-error and swallowed (asymmetric depth). SourceFileIndexer.kt:160 and :193
Both catch sites check cancellation only at the top level (e is CancellationException) while isMissingClasspathFile scans 32 causes deep. The FIR layer wraps exceptions in KotlinIllegalArgumentExceptionWithAttachments — the exact mechanism this fix relies on. If a ProcessCanceledException (which is-a CancellationException) is wrapped that same way during a cancelled resolve, the top-level check is false, isMissingClasspathFile is false, so it's reported to Sentry as an error and swallowed via return — breaking structured-concurrency cancellation and adding false error noise. Fix: scan the cause chain for CancellationException too, before the isMissingClasspathFile check.

3. Session-setup skip leaves the index inconsistent after removeBySource. SourceFileIndexer.kt:169
removeBySource(newFile.filePath) (line 125) commits before the try. If analyze() session setup then throws the transient missing-jar error, the catch returns without upserting — so symbolsIndex is now empty for the file while fileIndex still reports isIndexed=true with dangling symbolKeys. Recovery only happens on the next edit or a full refreshSources()/reopen scan (a rescan does recover it, since the stale stamps are older), not automatically when the jar returns. The comment "the file retries next time" overstates this.

4. A rebuilt (truncated) jar throws ZipException/EOFException, which isMissingClasspathFile misses. SourceFileIndexer.kt:75
The ticket names a transform-cache jar "evicted or rebuilt." Eviction -> NoSuchFileException (matched). Rebuild-in-progress -> the file exists but is half-written, so ZipFile/inflater throws java.util.zip.ZipException or EOFException — isMissingClasspathFile returns false and it's reported to Sentry as an error, i.e. exactly the benign-transient noise the fix set out to silence. Consider matching those zip/EOF types too.

5. Persistent (non-transient) missing-classpath is indistinguishable from transient and swallowed at debug forever. SourceFileIndexer.kt:163
A real misconfiguration that permanently references a missing jar hits NoSuchFileException on every pass, routing to logger.debug with no Sentry and no user signal. Combined with #1, the file stays partially/never indexed with nothing to escalate a condition that never clears. (Low; a design tradeoff, but worth a bounded-retry or escalate-after-N thought.)

6. The cause walk covers .cause but not .suppressed. SourceFileIndexer.kt:73
A NoSuchFileException delivered as a suppressed exception (e.g. a use{}/try-with-resources close failure while another exception is in flight) is never visited, so the benign case is reported as an error. Lower probability than the direct-cause case. (The .take(32) bound does correctly guard the cyclic-cause case.)

Altitude / fix depth

7. This is a per-call-site catch, not a scope-level backstop — sibling paths on the same handler-less scope still crash. KtSymbolIndex.kt:59 (fix boundary at SourceFileIndexer.kt:137/172)
The root cause is that KtSymbolIndex.scope uses SupervisorJob() with no CoroutineExceptionHandler (and CompilationEnvironment already defines exactly such a handler at CompilationEnvironment.kt:73-83 but never passes it through). The fix guards one origin; still uncaught on the same scope: toMetadata + the tail insertAll/upsert (both outside the new try in indexSourceFile), and the entire ScanSourceFile branch in IndexWorker.kt:132 (toMetadata + DB writes in the bare while(isActive) loop). A CoroutineExceptionHandler on KtSymbolIndex.scope would be the general fix that also covers these.

Test coverage

8. The crash-fix control flow itself is untested. SourceFileIndexerTest.kt:97+
The 5 new tests only exercise the pure isMissingClasspathFile predicate. Nothing asserts that a missing-classpath Throwable is swallowed and indexSourceFile returns without crashing, or that a genuine error is reported. The predicate can be perfect while the try/catch that consumes it is deleted, and every test still passes. The fixture supports injecting a real failure (a source file referencing a lib jar that is then removed).

9. No test that CancellationException is rethrown rather than swallowed. SourceFileIndexerTest.kt:97+
Trivially injectable: pass ICancelChecker.CANCELLED (its abortIfCancelled() throws inside the try) instead of NOOP and assert the call propagates. Guards against a future reorder/removal of the catch (CancellationException) branch — which would otherwise silently break cancellation and spam Sentry.

10. The pure-predicate tests carry the heavy KtLspTest Analysis-API fixture. SourceFileIndexerTest.kt:16
isMissingClasspathFile is internal in this package; the five predicate tests need no compilation environment, yet extending KtLspTest bootstraps a full MockProject + stdlib/JDK/kt-android.jar modules per method — and findIntellijPluginRoot() can error(...) if kt-android.jar isn't on the classpath, failing trivial tests for an unrelated reason. A plain test class would run them at zero env cost.

Cleanup (low)

11. New module logger uses a hardcoded FQN string. SourceFileIndexer.kt:59-62 — getLogger("com.itsaky.androidide.lsp.kotlin.compiler.index.SourceFileIndexer") matches neither the class-scoped peers (getLogger(IndexWorker::class.java)) nor the short-string peers (getLogger("KotlinCompletions")); it duplicates the package line and goes stale on a rename/move.

12. The rethrow-cancellation -> isMissingClasspathFile -> Sentry triage is copy-pasted at two layers (:191-198 and :160-168). The two-layer defense is defensible, but the identical three-step idiom could be one helper so a future policy change edits one place. (If you address #2 and #4, that logic changes in both spots — a shared helper makes that a single edit.)

---

Automated review via Claude Code (/code-review xhigh). Findings ranked most-severe first; correctness verified against the enclosing functions and call sites (IndexWorker, KtSymbolIndex, KeyedDebouncingAction, shouldBeSkipped).