scalafix icon indicating copy to clipboard operation
scalafix copied to clipboard

Insufficient/Unactionable error message.

Open Prithvirajbilla opened this issue 6 years ago • 9 comments

When I run scalafix against my whole repo, I am getting the following error

error: Unable to load symbol table: error in opening zip file

It would be nice to know which zip file (name, path) it is failing to open.

Thanks

Prithvirajbilla avatar May 06 '19 13:05 Prithvirajbilla

@Prithvirajbilla Could you add

  • the actual command you run when seeing this error
  • scalafix dependency version

so one can reproduce and fix this.

exoego avatar Aug 04 '19 08:08 exoego

I tracked down the error and it is actually an issue with scalameta, I think.

To reproduce this error, do the following

touch dll.dylib
scalafix --classpath=dll.dylib

Actual use case: I was running scalafix command line tool, passing classpath argument with a huge list of files(which I obtained from a build tool) And some of the files in that list or not actually jar files, but are .dylib files. And scalafix error messages wasn't very helpful when there is a long list of files in classpath.

Prithvirajbilla avatar Aug 06 '19 17:08 Prithvirajbilla

Sorry for bumping this error. I'll ping @bjaglin because it seems like he handled the resolution of related issues back when they were fresh.

We stumbled upon this issue at work. It was happening in a mill build but that's irrelevant. I found what was the issue, still it was very hard to find out what was wrong. There are a few reasons for this:

  1. the issue happened after an update to an internal plugin that shuffled with the classpath
  2. on disk, the directories/files layout of the input to scalafix was EXACTLY the same
  3. scalafix has no --debug or such option to print more info
  4. in some cases if it sees an error, it grabs the message but discard the stack trace, hence: Unable to load symbol table: error in opening zip file when you could get much more mileage out of a stack trace

I propose the following to address any further issue

  1. add a verbose (or debug) option to ScalafixArguments
  2. when said option is enabled, errors should be sent to the configured printstream
  3. when the classpath contains invalid things, the error should be explicit (validate classpath contents before passing it to scala meta for scanning)

wrt to 3), the javadoc says:

Class path entries that are neither directories nor archives (.zip or JAR files) nor the asterisk (*) wildcard character are ignored.

See https://docs.oracle.com/javase/8/docs/technotes/tools/windows/classpath.html, so I suggest we error out on such instance (or print a warning (all the time or in debug only, I don't mind) when it happens). We could support directories, jars (and zip) and semanticdb files.

daddykotex avatar Jul 20 '22 17:07 daddykotex

I'd be happy to implement the required changes to make that happen. I just want to have a plan before diving in.

daddykotex avatar Jul 20 '22 17:07 daddykotex

Good work on the investigation, I imagine it must have been painful... I haven't worked on this recently, but rather on stack trace suppression in sbt-scalafix: https://github.com/scalacenter/sbt-scalafix/pull/306.

  • (3) looks like the most impactful and actionable improvement to me. Thanks for volunteering, I'll be happy to help you on gitter if you have any questions while looking at it.
  • I am unsure about the ROI of (1)/(2) in that particular case as the exception is raised while parsing the arguments themselves https://github.com/scalacenter/scalafix/blob/9533c97acb926dea403265ef72d101f3057c728c/scalafix-cli/src/main/scala/scalafix/internal/v1/Args.scala#L210 As a quick win, I assume we could dump the stack trace to stderr unconditionally?

bjaglin avatar Jul 20 '22 22:07 bjaglin

(3) looks like the most impactful and actionable improvement to me. Thanks for volunteering, I'll be happy to help you on gitter if you have any questions while looking at it

All right, but before I do. Should i:

  • error out ? (if the classpath contains irrelevant entries, I just throw (or return .notOk, w/e is used in this part of the code)
  • filter out and print a warning to stdout?

As a quick win, I assume we could dump the stack trace to stderr unconditionally? yes, totally, I just did not know if this was a route we could take

daddykotex avatar Jul 21 '22 12:07 daddykotex

I interpreted (3) as: keep the current behavior (notOk), but with an actionable error message pointing out the faulty element in the classpath.

bjaglin avatar Jul 21 '22 13:07 bjaglin

FTR, here is the full stack trace

java.util.zip.ZipException: zip file is empty
        at java.base/java.util.zip.ZipFile$Source.zerror(ZipFile.java:1607)
        at java.base/java.util.zip.ZipFile$Source.findEND(ZipFile.java:1410)
	at java.base/java.util.zip.ZipFile$Source.initCEN(ZipFile.java:1504)
	at java.base/java.util.zip.ZipFile$Source.<init>(ZipFile.java:1308)
	at java.base/java.util.zip.ZipFile$Source.get(ZipFile.java:1271)
	at java.base/java.util.zip.ZipFile$CleanableResource.<init>(ZipFile.java:733)
	at java.base/java.util.zip.ZipFile$CleanableResource.get(ZipFile.java:850)
	at java.base/java.util.zip.ZipFile.<init>(ZipFile.java:248)
	at java.base/java.util.zip.ZipFile.<init>(ZipFile.java:177)
	at java.base/java.util.jar.JarFile.<init>(JarFile.java:350)
	at java.base/java.util.jar.JarFile.<init>(JarFile.java:321)
	at java.base/java.util.jar.JarFile.<init>(JarFile.java:287)
	at scala.meta.internal.classpath.ClasspathIndex$Builder.scala$meta$internal$classpath$ClasspathIndex$Builder$$expandJarEntry(ClasspathIndex.scala:136)
	at scala.meta.internal.classpath.ClasspathIndex$Builder.expandEntry(ClasspathIndex.scala:102)
	at scala.meta.internal.classpath.ClasspathIndex$Builder.$anonfun$result$1(ClasspathIndex.scala:64)
	at scala.meta.internal.classpath.ClasspathIndex$Builder.$anonfun$result$1$adapted(ClasspathIndex.scala:64)
	at scala.collection.immutable.List.foreach(List.scala:333)
	at scala.meta.internal.classpath.ClasspathIndex$Builder.result(ClasspathIndex.scala:64)
	at scala.meta.internal.classpath.ClasspathIndex$.apply(ClasspathIndex.scala:53)
	at scala.meta.internal.symtab.GlobalSymbolTable.<init>(GlobalSymbolTable.scala:21)
	at scala.meta.internal.symtab.GlobalSymbolTable$.apply(GlobalSymbolTable.scala:78)
	at scalafix.internal.reflect.ClasspathOps$.newSymbolTable(ClasspathOps.scala:33)

bjaglin avatar Jul 25 '22 18:07 bjaglin

For future uses that ends up here: in future versions of scala-meta/meta, invalid classpath entry will be ignored instead of raising a ZipException error. This means that this kind of error: java.util.zip.ZipException: zip file is empty should disappear

daddykotex avatar Aug 24 '22 17:08 daddykotex