Merge pull request #30 from gradle/pshevche/fix-build-result-exceptio…

…n-handling Do not try to create proxies for instances of `Throwable`
gradle · Apr 17, 2024 · 5e1fdbd · 5e1fdbd
2 parents ce44047 + cede201
commit 5e1fdbd
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 6 deletions.
diff --git a/buildSrc/src/main/kotlin/develocity/adapters/develocity.adapters-library.gradle.kts b/buildSrc/src/main/kotlin/develocity/adapters/develocity.adapters-library.gradle.kts
@@ -117,6 +117,8 @@ publishing {
 }
 
 signing {
+    isRequired = providers.environmentVariable("CI").isPresent
+
     sign(publishing.publications["mavenJava"])
     useInMemoryPgpKeys(System.getenv("PGP_SIGNING_KEY"), System.getenv("PGP_SIGNING_KEY_PASSPHRASE"))
 }
diff --git a/...patibilityApi/java/com/gradle/develocity/agent/gradle/adapters/internal/ProxyFactory.java b/...patibilityApi/java/com/gradle/develocity/agent/gradle/adapters/internal/ProxyFactory.java
@@ -40,7 +40,7 @@ public Object invoke(Object proxy, Method method, Object[] args) {
                 targetMethod.setAccessible(true);
 
                 Object result = targetMethod.invoke(target, targetArgs);
-                if (result == null || isJdkType(result.getClass())) {
+                if (result == null || isJdkTypeOrThrowable(result.getClass())) {
                     return result;
                 }
                 return createProxy(result, method.getReturnType());
@@ -62,7 +62,7 @@ private static Object[] toTargetArgs(Object[] args) {
             if (args.length == 1 && args[0] instanceof Function) {
                 return new Object[]{adaptFunctionArg((Function<?, ?>) args[0])};
             }
-            if (Arrays.stream(args).allMatch(it -> isJdkType(it.getClass()))) {
+            if (Arrays.stream(args).allMatch(it -> isJdkTypeOrThrowable(it.getClass()))) {
                 return args;
             }
             throw new RuntimeException("Unsupported argument types in " + Arrays.toString(args));
@@ -84,7 +84,7 @@ private static Function<Object, Object> adaptFunctionArg(Function func) {
         }
 
         private static Object createLocalProxy(Object target) {
-            if (isJdkType(target.getClass())) {
+            if (isJdkTypeOrThrowable(target.getClass())) {
                 return target;
             }
 
@@ -118,7 +118,7 @@ private static Class<?>[] convertTypes(Class<?>[] parameterTypes, ClassLoader cl
             }
             return Arrays.stream(parameterTypes)
                 .map(type -> {
-                    if (isJdkType(type)) {
+                    if (isJdkTypeOrThrowable(type)) {
                         return type;
                     }
 
@@ -131,9 +131,11 @@ private static Class<?>[] convertTypes(Class<?>[] parameterTypes, ClassLoader cl
                 .toArray(Class<?>[]::new);
         }
 
-        private static boolean isJdkType(Class<?> type) {
+        private static boolean isJdkTypeOrThrowable(Class<?> type) {
             ClassLoader typeClassLoader = type.getClassLoader();
-            return typeClassLoader == null || typeClassLoader.equals(Object.class.getClassLoader());
+            // JDK types are present across classloaders, so there is no need to create proxies for those
+            // we can't create proxies for instances of Throwable as there is no shared interface
+            return typeClassLoader == null || typeClassLoader.equals(Object.class.getClassLoader()) || Throwable.class.isAssignableFrom(type);
         }
 
     }

diff --git a/...gradle/develocity/agent/gradle/adapters/develocity/BuildScanConfigurationAdapterTest.java b/...gradle/develocity/agent/gradle/adapters/develocity/BuildScanConfigurationAdapterTest.java
@@ -186,6 +186,35 @@ public List<Throwable> getFailures() {
         assertEquals(Collections.singletonList(failure), capturedNewBuildResult.getValue().getFailures());
     }
 
+    @Test
+    @DisplayName("can run the build finished action using the proxy when the build result has custom failures")
+    @SuppressWarnings("Convert2Lambda")
+    void testBuildFinishedActionWithCustomException() {
+        // given
+        class CustomException extends Throwable {
+            public CustomException(String message) {
+                super(message);
+            }
+        }
+
+        // and
+        Throwable failure = new CustomException("Boom!");
+        BuildResult buildResult = new BuildResult() {
+            @Override
+            public List<Throwable> getFailures() {
+                return Collections.singletonList(failure);
+            }
+        };
+        doExecuteActionWith(buildResult).when(configuration).buildFinished(any());
+
+        // when
+        ArgCapturingAction<BuildResultAdapter> capturer = new ArgCapturingAction<>();
+        adapter.buildFinished(capturer);
+
+        // then
+        assertEquals(Collections.singletonList(failure), capturer.getValue().getFailures());
+    }
+
     @Test
     @DisplayName("build scan published action can be configured via an adapter using the new scan model")
     void testBuildScanPublishedAction() {

diff --git a/...com/gradle/develocity/agent/gradle/adapters/enterprise/BuildScanExtensionAdapterTest.java b/...com/gradle/develocity/agent/gradle/adapters/enterprise/BuildScanExtensionAdapterTest.java
@@ -178,6 +178,35 @@ public Throwable getFailure() {
         assertEquals(Collections.singletonList(failure), capturer.getValue().getFailures());
     }
 
+    @Test
+    @DisplayName("can run the build finished action using the proxy when the build result has custom failures")
+    @SuppressWarnings("Convert2Lambda")
+    void testBuildFinishedActionWithCustomException() {
+        // given
+        class CustomException extends Throwable {
+            public CustomException(String message) {
+                super(message);
+            }
+        }
+
+        // and
+        Throwable failure = new CustomException("Boom!");
+        BuildResult buildResult = new BuildResult() {
+            @Override
+            public Throwable getFailure() {
+                return failure;
+            }
+        };
+        doExecuteActionWith(buildResult).when(extension).buildFinished(any());
+
+        // when
+        ArgCapturingAction<BuildResultAdapter> capturer = new ArgCapturingAction<>();
+        adapter.buildFinished(capturer);
+
+        // then
+        assertEquals(Collections.singletonList(failure), capturer.getValue().getFailures());
+    }
+
     @Test
     @DisplayName("can run the build scan published action using the proxy")
     void testBuildScanPublishedAction() {