ADFA-3364: Skip corrupt JARs during classpath indexing (+ surface to user)#1410
ADFA-3364: Skip corrupt JARs during classpath indexing (+ surface to user)#1410fryanpan wants to merge 4 commits into
Conversation
|
Warning Review limit reached
More reviews will be available in 27 minutes and 37 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the 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 credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ 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 |
Sentry APPDEVFORALL-Y8. Catch ZipException/IOException per jar and continue instead of aborting the indexing job. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Feeds a truncated jar, a zero-byte jar, and a valid android.jar (in that order) to JarFsClasspathReader.listClasses. On the pre-fix baseline the ZipException from the corrupt jar propagated and aborted indexing; with the fix the corrupt entries are skipped and the valid jar is still fully indexed. Verified: RED on 4d9b100 (ZipException), GREEN on the fix branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…path Beyond skipping a corrupt jar during indexing, collect the skipped jars (JarFsClasspathReader.unreadableJars), expose them per-module (ModuleProject.unreadableClasspathJars), and after indexing show the user a Flashbar naming the offending dependency and pointing to Sync to re-download — instead of silently dropping that library's code-completion symbols. Layering: low-level reader/model only COLLECT; ProjectManagerImpl (which already uses flashError) does the user-facing report, so no UI reach-down from the model. Test: JarFsClasspathReader collects corrupt.jar + empty.jar but not valid.jar. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add override KDoc to JarFsClasspathReader.listClasses describing the corrupt-JAR skip + unreadableJars reporting behavior. Other changed symbols (unreadableJars, unreadableClasspathJars, reportUnreadableClasspathJars) already carried KDoc. Also apply the known CodeRabbit "major" defensive-copy fix: unreadableJars getter now returns _unreadableJars.toList() instead of the mutable backing list. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
aa6d28b to
6c0c0fc
Compare
Daniel-ADFA
left a comment
There was a problem hiding this comment.
Approving with nitpick
| val shown = names.take(3).joinToString(", ") | ||
| val more = if (names.size > 3) " and ${names.size - 3} more" else "" | ||
| log.warn("Skipped {} unreadable classpath JAR(s) during indexing: {}", names.size, names) | ||
| flashError( |
There was a problem hiding this comment.
please move user facing string to resource/string.xml
| flashError( | |
| flashError(R.string.xxx) |
Jira Ticket: https://appdevforall.atlassian.net/browse/ADFA-3364
Sentry Issue: https://appdevforall-inc-9p.sentry.io/issues/APPDEVFORALL-Y8
Reproduction Details
JarFsClasspathReader.listClassesopened each classpath entry as a JAR filesystem and walked it with no error handling. A single truncated/zero-byte/corrupt.jar(e.g. an interrupted download / incomplete offline provisioning) throwsjava.util.zip.ZipException, which propagated out and aborted indexing of every remaining (valid) classpath entry.Stack Trace
(device: vivo Y22s · device.class: medium · Android 14)
User Steps
User steps leading up to crash, based on Sentry breadcrumbs:
ModuleProject.indexClasspaths) and walks each dependency jar.ZipExceptionaborts the whole indexing pass.Was able to reproduce in a unit test?
Yes.
JarFsClasspathReaderCorruptJarTest(:subprojects:projects) feeds[corrupt.jar (64-byte truncated), empty.jar (0 bytes), valid android.jar]and assertsandroid.content.Contextis still indexed (>100 classes). Baseline: FAILS withZipException; branch: passes. A second test asserts the skipped jars are collected (unreadableJars= corrupt.jar + empty.jar, not valid.jar).What Was Fixed
walkFileTreeintry { … } catch (ZipException) { log.warn } catch (IOException) { log.warn }so a bad jar is skipped and indexing continues.JarFsClasspathReader.unreadableJars→ModuleProject.unreadableClasspathJars) and, after indexing, surface a user-visible Flashbar fromProjectManagerImplnaming the dependency + a recovery path ("Sync the project to re-download dependencies") instead of a silent skip. Layering preserved (low-level reader only collects; orchestrator reports).Testing
:subprojects:projects:testV8DebugUnitTest --tests …JarFsClasspathReaderCorruptJarTest→ 2/2 green (red on baseline). Local CodeRabbit review: 1 major — theunreadableJarsgetter should return a defensive copy (_unreadableJars.toList()) to avoid handing out the backing list that's cleared on the next call. (Corroborated by local review; will address.)Fixes APPDEVFORALL-Y8