Keep ClassNotFoundException as cause when reporting function not found. (GoogleCloudPlatform#26)

eamonnmcmanus · web-flow · commit 809d05a3b125 · 2020-04-13T14:39:26.000-07:00
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.
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
@@ -22,14 +22,17 @@ public class FunctionLoader<T extends CloudFunction> {
   private final String functionTarget;
   private final ClassLoader classLoader;
   private final FunctionSignatureMatcher<T> matcher;
+  private final ClassNotFoundException newStyleException;
 
   public FunctionLoader(
       String functionTarget,
       ClassLoader classLoader,
-      FunctionSignatureMatcher<T> matcher) {
+      FunctionSignatureMatcher<T> 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
@@ -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;
@@ -250,34 +251,48 @@ public void startServer() throws Exception {
     servletContextHandler.setContextPath("/");
     server.setHandler(NotFoundHandler.forServlet(servletContextHandler));
 
-    Optional<Class<?>> functionClass = loadFunctionClass();
+    ClassOrException functionClass = loadFunctionClass();
 
     HttpServlet servlet;
     if ("http".equals(functionSignatureType)) {
-      if (functionClass.isPresent()) {
-        servlet = NewHttpFunctionExecutor.forClass(functionClass.get());
-      } else {
-        FunctionLoader<HttpCloudFunction> 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<HttpCloudFunction> 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<BackgroundCloudFunction> 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<BackgroundCloudFunction> 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(
@@ -294,18 +309,44 @@ public void startServer() throws Exception {
     server.join();
   }
 
-  private Optional<Class<?>> 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
@@ -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<BackgroundCloudFunction> 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
@@ -45,7 +45,7 @@ public void adder() throws Exception {
         "com.google.cloud.functions.invoker.HttpFunctionTest$HttpWriter.writeResponse";
     String requestData = "testData";
     FunctionLoader<HttpCloudFunction> 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);
