jonase / eastwood

Clojure lint tool
1.08k stars 66 forks source link

Eastwood looks for optional class in snowplow and ClassNotFoundException occurs #444

Closed markbastian closed 1 year ago

markbastian commented 1 year ago

The snowplow-java-tracker library has the option of using either the okhttp or apache http clients. When the apache client is used, the code paths for the okhttp client are safely ignored.

When eastwood analyzes the snowplow code, it picks up the okhttp3.CookieJar class and tries to load it, resulting in a ClassNotFound exception.

When we actually use the library with the apache dependency, we don't encounter any issues at all.

I've got a small repo that reproduces the issue here.

dpsutton commented 1 year ago

A bit more context.

Snowplow allows using okhttpSupportApi 'com.squareup.okhttp3:okhttp:4.9.3' or apachehttpSupportApi 'org.apache.httpcomponents:httpclient:4.5.13' for network requests. It does not bundle either and requires you to supply the implementation. But the class has a reference to the private okhttp3.CookieJar cookieJar type. It seems fine for this type to be unknown at runtime, but eastwood errors that it cannot find a class for this when analyzing.

from it's pom:


  <modelVersion>4.0.0</modelVersion>
  <groupId>com.snowplowanalytics</groupId>
  <artifactId>snowplow-java-tracker</artifactId>
  <version>1.0.0</version>
  <name>snowplow-java-tracker</name>

...

    <dependency>
      <groupId>com.squareup.okhttp3</groupId>
      <artifactId>okhttp</artifactId>
      <version>4.9.3</version>
      <scope>compile</scope>
      <optional>true</optional>
    </dependency>
    <dependency>
      <groupId>org.apache.httpcomponents</groupId>
      <artifactId>httpclient</artifactId>
      <version>4.5.13</version>
      <scope>compile</scope>
      <optional>true</optional>
    </dependency>

when configuring the network adapter, you can specify how to create network requests. javadoc

Create a NetworkConfiguration instance and specify a custom HttpClientAdapter to use (the default is OkHttpClientAdapter).

There's an optional method cookieJar(okhttp3.CookieJar cookieJar) to set a cookjar of type okhttp3.CookieJar. We opt to not use this cookie jar nor the okhttp implementation and instead use the apache http client. Everything works fine at runtime. It's just that eastwood will analyze this class and error.

{:clojure.main/message
 "Execution error (ClassNotFoundException) at jdk.internal.loader.BuiltinClassLoader/loadClass (BuiltinClassLoader.java:581).\nokhttp3.CookieJar\n",
 :clojure.main/triage
 {:clojure.error/class java.lang.ClassNotFoundException,
  :clojure.error/line 581,
  :clojure.error/cause "okhttp3.CookieJar",
  :clojure.error/symbol jdk.internal.loader.BuiltinClassLoader/loadClass,
  :clojure.error/source "BuiltinClassLoader.java",
  :clojure.error/phase :execution},
 :clojure.main/trace
 {:via
  [{:type java.lang.NoClassDefFoundError,
    :message "Lokhttp3/CookieJar;",
    :at [java.lang.Class getDeclaredFields0 "Class.java" -2]}
   {:type java.lang.ClassNotFoundException,
    :message "okhttp3.CookieJar",
    :at [jdk.internal.loader.BuiltinClassLoader loadClass "BuiltinClassLoader.java" 581]}],
  :trace
  [[jdk.internal.loader.BuiltinClassLoader loadClass "BuiltinClassLoader.java" 581]
   [jdk.internal.loader.ClassLoaders$AppClassLoader loadClass "ClassLoaders.java" 178]
   [java.lang.ClassLoader loadClass "ClassLoader.java" 522]
   [java.lang.Class getDeclaredFields0 "Class.java" -2]
   [java.lang.Class privateGetDeclaredFields "Class.java" 3061]
   [java.lang.Class getDeclaredFields "Class.java" 2248]
   [clojure.reflect$declared_fields invokeStatic "java.clj" 170]
   [clojure.reflect.JavaReflector do_reflect "java.clj" 183]
   [clojure.reflect$fn__12007$G__12003__12010 invoke "reflect.clj" 44]
   [clojure.reflect$fn__12007$G__12002__12014 invoke "reflect.clj" 44]
   [clojure.core$partial$fn__5908 invoke "core.clj" 2641]
   [clojure.reflect$type_reflect invokeStatic "reflect.clj" 100]
   [clojure.reflect$type_reflect doInvoke "reflect.clj" 58]
   [clojure.lang.RestFn applyTo "RestFn.java" 139]
   [clojure.core$apply invokeStatic "core.clj" 673]
   [clojure.core$apply invoke "core.clj" 662]
   [eastwood.copieddeps.dep2.clojure.tools.analyzer.jvm.utils$type_reflect invokeStatic "utils.clj" 21]
   [eastwood.copieddeps.dep2.clojure.tools.analyzer.jvm.utils$type_reflect doInvoke "utils.clj" 19]
   [clojure.lang.RestFn invoke "RestFn.java" 439]
   [eastwood.copieddeps.dep2.clojure.tools.analyzer.jvm.utils$members_STAR___10944 invokeStatic "utils.clj" 272]
   [eastwood.copieddeps.dep2.clojure.tools.analyzer.jvm.utils$members_STAR___10944 invoke "utils.clj" 262]
   [clojure.lang.AFn applyToHelper "AFn.java" 154]
   [clojure.lang.AFn applyTo "AFn.java" 144]
   [clojure.core$apply invokeStatic "core.clj" 667]
   [clojure.core$apply invoke "core.clj" 662]
   [eastwood.copieddeps.dep3.clojure.core.memoize$through_STAR_$fn__10746 invoke "memoize.clj" 110]
   [eastwood.copieddeps.dep4.clojure.core.cache$through$fn__10476 invoke "cache.clj" 55]
   [eastwood.copieddeps.dep3.clojure.core.memoize$through_STAR_$fn__10742$fn__10743 invoke "memoize.clj" 109]
   [eastwood.copieddeps.dep3.clojure.core.memoize.RetryingDelay deref "memoize.clj" 47]
   [clojure.core$deref invokeStatic "core.clj" 2337]
   [clojure.core$deref invoke "core.clj" 2323]
   [eastwood.copieddeps.dep3.clojure.core.memoize$cached_function$fn__10810 doInvoke "memoize.clj" 234]
   [clojure.lang.RestFn applyTo "RestFn.java" 137]
   [clojure.lang.AFunction$1 doInvoke "AFunction.java" 31]
   [clojure.lang.RestFn invoke "RestFn.java" 408]
   [eastwood.copieddeps.dep2.clojure.tools.analyzer.jvm.utils$members invokeStatic "utils.clj" 279]
   [eastwood.copieddeps.dep2.clojure.tools.analyzer.jvm.utils$members invoke "utils.clj" 275]
   [eastwood.copieddeps.dep2.clojure.tools.analyzer.passes.jvm.validate$eval11671$fn__11673 invoke "validate.clj" 72]
   [clojure.lang.MultiFn invoke "MultiFn.java" 229]
   [eastwood.copieddeps.dep2.clojure.tools.analyzer.passes.jvm.validate$validate invokeStatic "validate.clj" 265]
   [eastwood.copieddeps.dep2.clojure.tools.analyzer.passes.jvm.validate$validate invoke "validate.clj" 240]
...
vemv commented 1 year ago

Thank you both!

a tldr from the Slack thread is:

Just surprised that a class can reference types not on the classpath as long as they are not referenced at runtime

We'd have to double-check that that is the case, as it indeed would be (very) surprising behavior. Does something like lein deps :tree or manually inspecting the uberjar show that the optional class is 100% absent? Maybe it's being pulled anyway for some reason.

Besides from the definterface workaround, a no-code solution would be to run eastwood within an :eastwood profile/alias that includes the optional .jar. That seems quite reasonable to me since import okhttp3.CookieJar; is a hardcoded statement.

Before thinking of any fix for the Eastwood side, we'd have to determine the actual java/javac/jvm behavior. I'd ask you to inspect trees/uberjars so that we can proceed.

Cheers - V

vemv commented 1 year ago

Does something like lein deps :tree or manually inspecting the uberjar show that the optional class is 100% absent?

(You can also check this in a prod-like repl by simply typing okhttp3.CookieJar)

markbastian commented 1 year ago

clj -Stree produces the following with respect to the snowplow-java-tracker library:

com.snowplowanalytics/snowplow-java-tracker 1.0.0
  X commons-codec/commons-codec 1.15 :use-top
  X commons-net/commons-net 3.6 :use-top
  X org.slf4j/slf4j-api 1.7.36 :use-top

I've also updated the above repo example, but what's interesting is this:

;; This works ok
(def network-config
  (let [request-config      (-> (RequestConfig/custom)
                                (.setCookieSpec CookieSpecs/STANDARD)
                                (.build))
        client              (-> (HttpClients/custom)
                                (.setConnectionManager (PoolingHttpClientConnectionManager.))
                                (.setDefaultRequestConfig request-config)
                                (.build))
        http-client-adapter (ApacheHttpClientAdapter. "http://localhost:9090" client)]
    (NetworkConfiguration. http-client-adapter)))

;; Then we eval the following
(comment
  network-config                             ;; Evaluating this works
  (.getCookieJar network-config)  ;; This blows up (can't find the class) rather than returning nil
  )

I agree, though, that the import statement you highlight seems like it ought to blow up with any sort of aot compilation or tasks that try to load all of the classes.

For now, I can just do :exclude-namespaces [metabase.analytics.snowplow] to get my own project working.

markbastian commented 1 year ago

A little more data. I created an aot'd uberjar and then exploded it with jar -xvf. Below is a screen grab. No okhttp3 classes.

image
markbastian commented 1 year ago

It really feels like this is a bug in the snowplow code and the safest thing to do is just exclude that file from being linted. They really should be using an interface rather than reaching for a 3rd party class that might be used.