From bdbe787ad05d7c5a7f3f18a410e53df9c4ac6e67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89amonn=20McManus?= Date: Mon, 13 Apr 2020 11:03:11 -0700 Subject: [PATCH] Keep ClassNotFoundException as cause when reporting function not found. The code is complicated by the support for legacy "ClassName.method" way of specifying function targets, which we will delete when we delete java8 support from GCF. Under the assumption that the user did mean to use the new form, we remember what the ClassNotFoundException was that caused us to fail, and we include that in the cause chain of the final exception. That way the user can see a possible ExceptionInInitializerError or the like that might be the underlying reason for the failure. --- .../functions/invoker/FunctionLoader.java | 10 ++- .../functions/invoker/runner/Invoker.java | 89 ++++++++++++++----- .../invoker/BackgroundFunctionTest.java | 7 +- .../functions/invoker/HttpFunctionTest.java | 2 +- 4 files changed, 78 insertions(+), 30 deletions(-) diff --git a/invoker/core/src/main/java/com/google/cloud/functions/invoker/FunctionLoader.java b/invoker/core/src/main/java/com/google/cloud/functions/invoker/FunctionLoader.java index c5b2dc08..509a5e3a 100644 --- a/invoker/core/src/main/java/com/google/cloud/functions/invoker/FunctionLoader.java +++ b/invoker/core/src/main/java/com/google/cloud/functions/invoker/FunctionLoader.java @@ -22,14 +22,17 @@ public class FunctionLoader { private final String functionTarget; private final ClassLoader classLoader; private final FunctionSignatureMatcher matcher; + private final ClassNotFoundException newStyleException; public FunctionLoader( String functionTarget, ClassLoader classLoader, - FunctionSignatureMatcher matcher) { + FunctionSignatureMatcher matcher, + ClassNotFoundException newStyleException) { this.functionTarget = functionTarget; this.classLoader = classLoader; this.matcher = matcher; + this.newStyleException = newStyleException; } /** @@ -40,7 +43,7 @@ public FunctionLoader( public T loadUserFunction() throws Exception { int lastDotIndex = functionTarget.lastIndexOf("."); if (lastDotIndex == -1) { - throw new ClassNotFoundException(functionTarget); + throw newStyleException; } String targetClassName = functionTarget.substring(0, lastDotIndex); String targetMethodName = functionTarget.substring(lastDotIndex + 1); @@ -50,7 +53,8 @@ public T loadUserFunction() throws Exception { } catch (ClassNotFoundException e) { throw new RuntimeException( "Could not load either " + functionTarget + " (new form) or " - + targetClassName + " (old form)"); + + targetClassName + " (old form)", + newStyleException); } Object targetInstance = targetClass.getDeclaredConstructor().newInstance(); diff --git a/invoker/core/src/main/java/com/google/cloud/functions/invoker/runner/Invoker.java b/invoker/core/src/main/java/com/google/cloud/functions/invoker/runner/Invoker.java index 3bcd4ba4..626f9c0c 100644 --- a/invoker/core/src/main/java/com/google/cloud/functions/invoker/runner/Invoker.java +++ b/invoker/core/src/main/java/com/google/cloud/functions/invoker/runner/Invoker.java @@ -19,6 +19,7 @@ import com.beust.jcommander.JCommander; import com.beust.jcommander.Parameter; import com.beust.jcommander.ParameterException; +import com.google.auto.value.AutoOneOf; import com.google.cloud.functions.BackgroundFunction; import com.google.cloud.functions.HttpFunction; import com.google.cloud.functions.RawBackgroundFunction; @@ -249,34 +250,48 @@ public void startServer() throws Exception { servletContextHandler.setContextPath("/"); server.setHandler(NotFoundHandler.forServlet(servletContextHandler)); - Optional> functionClass = loadFunctionClass(); + ClassOrException functionClass = loadFunctionClass(); HttpServlet servlet; if ("http".equals(functionSignatureType)) { - if (functionClass.isPresent()) { - servlet = NewHttpFunctionExecutor.forClass(functionClass.get()); - } else { - FunctionLoader loader = new FunctionLoader<>( - functionTarget, functionClassLoader, new HttpFunctionSignatureMatcher()); - HttpCloudFunction function = loader.loadUserFunction(); - servlet = new HttpFunctionExecutor(function); + switch (functionClass.getKind()) { + case LOADED_CLASS: + servlet = NewHttpFunctionExecutor.forClass(functionClass.loadedClass()); + break; + default: + FunctionLoader loader = + new FunctionLoader<>( + functionTarget, + functionClassLoader, + new HttpFunctionSignatureMatcher(), + functionClass.exception()); + HttpCloudFunction function = loader.loadUserFunction(); + servlet = new HttpFunctionExecutor(function); } } else if ("event".equals(functionSignatureType)) { - if (functionClass.isPresent()) { - servlet = NewBackgroundFunctionExecutor.forClass(functionClass.get()); - } else { - FunctionLoader loader = - new FunctionLoader<>( - functionTarget, functionClassLoader, new BackgroundFunctionSignatureMatcher()); - BackgroundCloudFunction function = loader.loadUserFunction(); - servlet = new BackgroundFunctionExecutor(function); + switch (functionClass.getKind()) { + case LOADED_CLASS: + servlet = NewBackgroundFunctionExecutor.forClass(functionClass.loadedClass()); + break; + default: + FunctionLoader loader = + new FunctionLoader<>( + functionTarget, + functionClassLoader, + new BackgroundFunctionSignatureMatcher(), + functionClass.exception()); + BackgroundCloudFunction function = loader.loadUserFunction(); + servlet = new BackgroundFunctionExecutor(function); } } else if (functionSignatureType == null) { - if (functionClass.isPresent()) { - servlet = servletForDeducedSignatureType(functionClass.get()); - } else { - throw new RuntimeException( - "Class " + functionTarget + " does not exist or could not be loaded"); + switch (functionClass.getKind()) { + case LOADED_CLASS: + servlet = servletForDeducedSignatureType(functionClass.loadedClass()); + break; + default: + throw new RuntimeException( + "Class " + functionTarget + " does not exist or could not be loaded", + functionClass.exception()); } } else { String error = String.format( @@ -291,18 +306,44 @@ public void startServer() throws Exception { server.join(); } - private Optional> loadFunctionClass() { + @AutoOneOf(ClassOrException.Kind.class) + abstract static class ClassOrException { + enum Kind {LOADED_CLASS, EXCEPTION} + + abstract Kind getKind(); + + abstract Class loadedClass(); + + abstract ClassNotFoundException exception(); + + static ClassOrException of(Class c) { + return AutoOneOf_Invoker_ClassOrException.loadedClass(c); + } + + static ClassOrException of(ClassNotFoundException e) { + return AutoOneOf_Invoker_ClassOrException.exception(e); + } + } + + private ClassOrException loadFunctionClass() { String target = functionTarget; + ClassNotFoundException firstException = null; while (true) { try { - return Optional.of(functionClassLoader.loadClass(target)); + return ClassOrException.of(functionClassLoader.loadClass(target)); } catch (ClassNotFoundException e) { + if (firstException == null) { + firstException = e; + } // This might be a nested class like com.example.Foo.Bar. That will actually appear as // com.example.Foo$Bar as far as Class.forName is concerned. So we try to replace every dot // from the last to the first with a $ in the hope of finding a class we can load. int lastDot = target.lastIndexOf('.'); if (lastDot < 0) { - return Optional.empty(); + // We're going to try to load the function target using the old form, classname.method. + // But it's at least as likely that the class does exist, but we failed to load it for + // some other reason. So ensure that we keep the exception to show to the user. + return ClassOrException.of(firstException); } target = target.substring(0, lastDot) + '$' + target.substring(lastDot + 1); } diff --git a/invoker/core/src/test/java/com/google/cloud/functions/invoker/BackgroundFunctionTest.java b/invoker/core/src/test/java/com/google/cloud/functions/invoker/BackgroundFunctionTest.java index f52071d6..7b0273cb 100644 --- a/invoker/core/src/test/java/com/google/cloud/functions/invoker/BackgroundFunctionTest.java +++ b/invoker/core/src/test/java/com/google/cloud/functions/invoker/BackgroundFunctionTest.java @@ -84,8 +84,11 @@ public void adder() throws Exception { HttpServletResponse res = Mockito.mock(HttpServletResponse.class); String fullTarget = "com.google.cloud.functions.invoker.BackgroundFunctionTest$" + target; FunctionLoader loader = - new FunctionLoader<>(fullTarget, getClass().getClassLoader(), - new BackgroundFunctionSignatureMatcher()); + new FunctionLoader<>( + fullTarget, + getClass().getClassLoader(), + new BackgroundFunctionSignatureMatcher(), + null); BackgroundCloudFunction function = loader.loadUserFunction(); BackgroundFunctionExecutor executor = new BackgroundFunctionExecutor(function); Mockito.when(req.getReader()) diff --git a/invoker/core/src/test/java/com/google/cloud/functions/invoker/HttpFunctionTest.java b/invoker/core/src/test/java/com/google/cloud/functions/invoker/HttpFunctionTest.java index 4f51315e..d1ee811d 100644 --- a/invoker/core/src/test/java/com/google/cloud/functions/invoker/HttpFunctionTest.java +++ b/invoker/core/src/test/java/com/google/cloud/functions/invoker/HttpFunctionTest.java @@ -45,7 +45,7 @@ public void adder() throws Exception { "com.google.cloud.functions.invoker.HttpFunctionTest$HttpWriter.writeResponse"; String requestData = "testData"; FunctionLoader loader = new FunctionLoader<>( - fullTarget, getClass().getClassLoader(), new HttpFunctionSignatureMatcher()); + fullTarget, getClass().getClassLoader(), new HttpFunctionSignatureMatcher(), null); HttpCloudFunction function = loader.loadUserFunction(); HttpFunctionExecutor executor = new HttpFunctionExecutor(function); Mockito.when(req.getParameter("data")).thenReturn(requestData);