You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2023/01/11 04:47:53 UTC

[GitHub] [netbeans] lkishalmi opened a new pull request, #5270: Catch IAE when Gradle error getLocation cannot be called.

lkishalmi opened a new pull request, #5270:
URL: https://github.com/apache/netbeans/pull/5270

   I've bumped into this one today:
   
   ```
   java.lang.IllegalArgumentException: object is not an instance of declaring class
   	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
   	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
   	at org.netbeans.modules.gradle.loaders.LegacyProjectLoader.getLocation(LegacyProjectLoader.java:380)
   ```
   
   Couldn't really know how to reproduce. I was fiddling with old Gradle versions with the new tooling.
   
   The fix is trivial though...


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5270: Catch IAE when Gradle error getLocation cannot be called.

Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #5270:
URL: https://github.com/apache/netbeans/pull/5270#discussion_r1066618591


##########
extide/gradle/src/org/netbeans/modules/gradle/loaders/LegacyProjectLoader.java:
##########
@@ -380,8 +380,10 @@ private static String getLocation(Throwable locationAwareEx) {
             return (String)locationAccessor.invoke(locationAwareEx);
         } catch (ReflectiveOperationException ex) {
             LOG.log(Level.FINE,"Error getting location", ex);
-            return null;
+        } catch (IllegalArgumentException iae) {
+            LOG.log(Level.FINE,"Trying to get location from an exception, that has no location property: " + locationAwareEx.getClass().getName());
         }
+        return null;

Review Comment:
   this illustrates the issue:
   ```java
           Integer five = 5;
           Double pi = Math.PI;
           
           Method stringAccessor =  five.getClass().getMethod("toString");
           System.out.println(stringAccessor.invoke(pi));
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on pull request #5270: Catch IAE when Gradle error getLocation cannot be called.

Posted by GitBox <gi...@apache.org>.
mbien commented on PR #5270:
URL: https://github.com/apache/netbeans/pull/5270#issuecomment-1378374703

   added do not merge label so that nobody merges by accident


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] lkishalmi commented on a diff in pull request #5270: Catch IAE when Gradle error getLocation cannot be called.

Posted by GitBox <gi...@apache.org>.
lkishalmi commented on code in PR #5270:
URL: https://github.com/apache/netbeans/pull/5270#discussion_r1066608548


##########
extide/gradle/src/org/netbeans/modules/gradle/loaders/LegacyProjectLoader.java:
##########
@@ -380,8 +380,10 @@ private static String getLocation(Throwable locationAwareEx) {
             return (String)locationAccessor.invoke(locationAwareEx);
         } catch (ReflectiveOperationException ex) {
             LOG.log(Level.FINE,"Error getting location", ex);
-            return null;
+        } catch (IllegalArgumentException iae) {
+            LOG.log(Level.FINE,"Trying to get location from an exception, that has no location property: " + locationAwareEx.getClass().getName());
         }
+        return null;

Review Comment:
   Well, I think it's a tradeoff we need to pay when we try to cover multiple versions at once. My mind could trick me, but some exceptions had `getLocation` methods even before `LocationAwareException` was introduced.
   
   This is just some heuristic, trying to figure out what's behind a Gradle build failure. If we can get a location out of that we display it as an editor hint on the source code.
   
   Well, if the heuristic does not work we just shall let it go...



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5270: Catch IAE when Gradle error getLocation cannot be called.

Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #5270:
URL: https://github.com/apache/netbeans/pull/5270#discussion_r1066613836


##########
extide/gradle/src/org/netbeans/modules/gradle/loaders/LegacyProjectLoader.java:
##########
@@ -380,8 +380,10 @@ private static String getLocation(Throwable locationAwareEx) {
             return (String)locationAccessor.invoke(locationAwareEx);
         } catch (ReflectiveOperationException ex) {
             LOG.log(Level.FINE,"Error getting location", ex);
-            return null;
+        } catch (IllegalArgumentException iae) {
+            LOG.log(Level.FINE,"Trying to get location from an exception, that has no location property: " + locationAwareEx.getClass().getName());
         }
+        return null;

Review Comment:
   but doesn't the `Method` field has to fit to the right class? If this method is supposed to be as generic as possible and work for all exceptions which have a `getLocation` method, I don't think it should be cached as field. It should be checked if that exception has `getLocation` on every invocation:
   
   ```java
               Method locationAccessor = locationAwareEx.getClass().getMethod("getLocation"); // NOI18N
               return (String)locationAccessor.invoke(locationAwareEx);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] lkishalmi commented on a diff in pull request #5270: Catch IAE when Gradle error getLocation cannot be called.

Posted by GitBox <gi...@apache.org>.
lkishalmi commented on code in PR #5270:
URL: https://github.com/apache/netbeans/pull/5270#discussion_r1066627608


##########
extide/gradle/src/org/netbeans/modules/gradle/loaders/LegacyProjectLoader.java:
##########
@@ -380,8 +380,10 @@ private static String getLocation(Throwable locationAwareEx) {
             return (String)locationAccessor.invoke(locationAwareEx);
         } catch (ReflectiveOperationException ex) {
             LOG.log(Level.FINE,"Error getting location", ex);
-            return null;
+        } catch (IllegalArgumentException iae) {
+            LOG.log(Level.FINE,"Trying to get location from an exception, that has no location property: " + locationAwareEx.getClass().getName());
         }
+        return null;

Review Comment:
   Point taken. Haven't really checked what's in the try block. Thanks!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] lkishalmi commented on pull request #5270: Catch IAE when Gradle error getLocation cannot be called.

Posted by GitBox <gi...@apache.org>.
lkishalmi commented on PR #5270:
URL: https://github.com/apache/netbeans/pull/5270#issuecomment-1379175786

   @sdedic Take your time!
   
   Just would like to add, that it's fine not to cache the Method accessor here. This is on a critical path.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on pull request #5270: Catch IAE when Gradle error getLocation cannot be called.

Posted by GitBox <gi...@apache.org>.
mbien commented on PR #5270:
URL: https://github.com/apache/netbeans/pull/5270#issuecomment-1380581487

   @sdedic thanks a lot for checking! Reflection is super fast anyway these days - I don't think much is lost there.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] lkishalmi merged pull request #5270: Catch IAE when Gradle error getLocation cannot be called.

Posted by GitBox <gi...@apache.org>.
lkishalmi merged PR #5270:
URL: https://github.com/apache/netbeans/pull/5270


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5270: Catch IAE when Gradle error getLocation cannot be called.

Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #5270:
URL: https://github.com/apache/netbeans/pull/5270#discussion_r1066596122


##########
extide/gradle/src/org/netbeans/modules/gradle/loaders/LegacyProjectLoader.java:
##########
@@ -380,8 +380,10 @@ private static String getLocation(Throwable locationAwareEx) {
             return (String)locationAccessor.invoke(locationAwareEx);
         } catch (ReflectiveOperationException ex) {
             LOG.log(Level.FINE,"Error getting location", ex);
-            return null;
+        } catch (IllegalArgumentException iae) {
+            LOG.log(Level.FINE,"Trying to get location from an exception, that has no location property: " + locationAwareEx.getClass().getName());
         }
+        return null;

Review Comment:
   the problem might be deeper than this. The method accepts throwable but expects it to be `LocationAwareException`. If someone would call it with the wrong exception type 
   ```java
               if (locationAccessor == null) {
                   locationAccessor = locationAwareEx.getClass().getMethod("getLocation"); // NOI18N
               }
   ```
   would intitialize the field wrong if it happens to have that method, and all subsequent calls will fail or do really weird things.
   
   I personally am not a fan of `Throwable` usage, since it is in 99% of all cases not something you should be catching.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] sdedic commented on pull request #5270: Catch IAE when Gradle error getLocation cannot be called.

Posted by GitBox <gi...@apache.org>.
sdedic commented on PR #5270:
URL: https://github.com/apache/netbeans/pull/5270#issuecomment-1378372819

   Let me try a different fix ... I've cached the accessor so that the reflective access is not that slow; but it should be easy to check for `instanceof LocationAwareException`. It seems that I might do a completely wrong thing with this reflective stuff, as the LocationAwareEx type is in the same place since Gradle 3.1 ... give me 24hrs to check, thanks.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] sdedic commented on pull request #5270: Catch IAE when Gradle error getLocation cannot be called.

Posted by GitBox <gi...@apache.org>.
sdedic commented on PR #5270:
URL: https://github.com/apache/netbeans/pull/5270#issuecomment-1380428682

   Ouch ! No caching is possible, in fact. During debugging, I've realized (only now) that the Tooling API seems to load classes for the deserialized data using a throwaway classloader , so Classes from 2 project loads are not assignable at all, even though they represent the same type. 
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists