Repository: square/retrofit
Issue: NPE when retrofit used by IntelliJ IDEA plugin
Labels: Enhancement

What kind of issue is this?

 - [ ] Question. This issue tracker is not the place for questions. If you want to ask how to do
       something, or to understand why something isn't working the way you expect it to, use Stack
       Overflow. https://stackoverflow.com/questions/tagged/retrofit

 - [X ] Bug report. If you’ve found a bug, spend the time to write a failing test. Bugs with tests
       get fixed. Here’s an example: https://gist.github.com/swankjesse/6608b4713ad80988cdc9

 - [ ] Feature Request. Start by telling us what problem you’re trying to solve. Often a solution
       already exists! Don’t send pull requests to implement new features without first getting our
       support. Sometimes we leave features out on purpose to keep the project small.

We are developing a plugin for IntelliJ IDEA Ultimate. The plugin uses retrofit for communication with a service. The plugin is crashing due to a NullPointerException resulting from retrofit's platform detection getting confused by the presence of an Android class on the IntelliJ IDEA classpath. The class is present due to IntelliJ's bundled Android Support plugin. The stack trace is:
```
2016-11-30 14:30:27,654 [   7389]  ERROR - plication.impl.ApplicationImpl - null
java.lang.ExceptionInInitializerError
        at java.lang.Class.forName0(Native Method)
        at java.lang.Class.forName(Class.java:264)
        at retrofit2.Platform.findPlatform(Platform.java:37)
        at retrofit2.Platform.<clinit>(Platform.java:29)
        at retrofit2.Retrofit$Builder.<init>(Retrofit.java:402)
        ...
        at com.intellij.openapi.application.impl.ApplicationImpl$3.call(ApplicationImpl.java:331)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
        at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.NullPointerException
        at android.os.SystemProperties_Delegate.native_get(SystemProperties_Delegate.java:44)
        at android.os.SystemProperties.native_get(SystemProperties.java)
        at android.os.SystemProperties.get(SystemProperties.java:64)
        at android.os.Build.getString(Build.java:819)
        at android.os.Build.<clinit>(Build.java:39)
        ... 13 more
```
The relevant portion of the retrofit code is from the Platform class:
```java
  private static Platform findPlatform() {
    try {
      Class.forName("android.os.Build");
      if (Build.VERSION.SDK_INT != 0) {
        return new Android();
      }
    } catch (ClassNotFoundException ignored) {
    }
    try {
      Class.forName("java.util.Optional");
      return new Java8();
    } catch (ClassNotFoundException ignored) {
    }
    return new Platform();
  }
```
The environment is RedHat Linux 6.6 running IntelliJ IDEA Ultimate 2016.3. That version of IntelliJ comes bundled with the Android Support plugin version 10.2.2.

Issue comments:
Submitted pull request #2103.
Any idea how I can look to see what classes from Android are included? We tend to like class lookups since they offer better insight into the capabilities of the enclosing VM rather than inspecting properties.

Or, more specifically, if we switched to checking for `android.os.Looper` would the same problem occur?
You would have to write a plugin and try to lookup the class you are considering to see if it throws. Even if it does not throw, I recommend against this approach because JetBrains / Google may choose to expose additional classes over time; there is no guarantee. In my experience the safest way to achieve what you want is to ask the JVM (e.g. properties) which is much more stable.
For feature detection inside a platform we've traditionally favored the opposite. But detection of the platform itself might mean needing to look at properties.

@swankjesse?
Curious – what if we instead of probing for `Build` we could probe for `Looper`? If that works it might mean we end up with the right behavior on Android, Java, Android Studio and Robolectric.
Hello, this issue is still blocking us to use retrofit in any environment that has android classes on classpath. Are there any plans to fix it?
using JVM properties will be way smarter way to find Platform.

Also it would be nice to give opportunity to user to provide platform. This will provide valid workaround for any usecase. For example just do System.getProperty("retrofit.platform")as first line and if property is provided create appropriate Platform, if not use default logic. This will allow lib clients to specify Platform if needed.

Thanks,
-Alex
Hey @JakeWharton / @swankjesse, are there any updates here? I'm running into an identical issue (plugin development for JetBrains IntelliJ) which is preventing me from using retrofit.

It looks like there hasn't been any movement on the PR (#2103) that @baron1405 submitted over a year ago.

**Edit:** Something like @akazancev is suggesting would work; if a system property is set, use the property, otherwise default to existing logic. :)

Looking forward to your thoughts!

Best,
Chris
@swankjesse do you have any updates on this?
Thanks,
-Alex

Pull request review comments:
This seems like enough of an indicator where we don't even need to bother with the `Class.forName` check?

@swankjesse thoughts?
Looks like `Dalvik` value will be there forever, even on ART it's `Dalvik` and `java.vm.version` can be used to distinguish between Dalvik/ART/etc https://developer.android.com/guide/practices/verifying-apps-art.html#GC_Migration

Would be great to apply this to OkHttp and RxJava if you'll decide to remove class lookup.
Will it work if we look for `android.os.Looper` instead? Seems like if that class is present then we should have Android behavior
BTW you can replace the forName check for Java 8 by using the `java.class.version` property. Any value >= 52 is Java 8 or newer.
I am concerned that attempting to load any Android class runs the risk of undesirable consequences especially in an IDE environment. Case in point is the android.os.Builder class was not on the classpath in the IDEA 2016.2 version of the Android Support plugin but has appeared on the classpath in the 2016.3 version. In other words, it is impossible to predict what Google and JetBrains will expose either intentionally or accidentally. A property based approach would be the safest. 
True. It also affects our ability to test this behavior since the Android classes are on the classpath but aren't actually usable.

Patch that resolved the issue:
diff --git a/retrofit/src/main/java/retrofit2/Platform.java b/retrofit/src/main/java/retrofit2/Platform.java
index ad9cf533c7..5340c7f80b 100644
--- a/retrofit/src/main/java/retrofit2/Platform.java
+++ b/retrofit/src/main/java/retrofit2/Platform.java
@@ -32,12 +32,14 @@ static Platform get() {
   }
 
   private static Platform findPlatform() {
-    try {
-      Class.forName("android.os.Build");
-      if (Build.VERSION.SDK_INT != 0) {
-        return new Android();
+    if ("Dalvik".equals(System.getProperty("java.vm.name"))) {
+      try {
+        Class.forName("android.os.Build");
+        if (Build.VERSION.SDK_INT != 0) {
+          return new Android();
+        }
+      } catch (ClassNotFoundException ignored) {
       }
-    } catch (ClassNotFoundException ignored) {
     }
     try {
       Class.forName("java.util.Optional");


Test patch: