diff --git a/api/src/main/java/com/fnproject/fn/api/FunctionInvoker.java b/api/src/main/java/com/fnproject/fn/api/FunctionInvoker.java index 7f1fa29a..6dd0a8ef 100644 --- a/api/src/main/java/com/fnproject/fn/api/FunctionInvoker.java +++ b/api/src/main/java/com/fnproject/fn/api/FunctionInvoker.java @@ -12,8 +12,11 @@ public interface FunctionInvoker { * If the invoker returns an empty option then no action has been taken and another invoker may attempt to tryInvoke *

* In particular this means that implementations must not read the input event body until they are committed to handling this event. - * - * A RuntimeException thrown by the implementation will cause the entire function invocation to fail. + *

+ * An {@link InputCoercion.InvalidFunctionInputException} thrown by the implementation will indicate that the input + * provided to the function by the user is invalid. + *

+ * Any other RuntimeException thrown by the implementation will cause the entire function invocation to fail. * * @param ctx the context for a single invocation * @param evt the incoming event diff --git a/api/src/main/java/com/fnproject/fn/api/InputCoercion.java b/api/src/main/java/com/fnproject/fn/api/InputCoercion.java index 1483092b..f546bff9 100644 --- a/api/src/main/java/com/fnproject/fn/api/InputCoercion.java +++ b/api/src/main/java/com/fnproject/fn/api/InputCoercion.java @@ -6,20 +6,39 @@ * Handles the coercion of an input event to a parameter */ public interface InputCoercion { + /** + * An implementor of InputCoercion can throw this exception to inform the platform that the user input in the http + * request was invalid. + */ + class InvalidFunctionInputException extends RuntimeException { + public InvalidFunctionInputException(String s, Throwable t) { + super(s, t); + } + + public InvalidFunctionInputException(String s) { + super(s); + } + } + /** * Handle coercion for a function parameter *

- * Coercions may choose to act on a parameter, in which case they should return a fulfilled option) or may + * Coercions may choose to act on a parameter, in which case they must return a fulfilled option) or may * ignore parameters (allowing other coercions to act on the parameter) *

- * When a coercion ignores a parameter it must not consume the input stream of the event. + * When a coercion ignores a parameter it must not consume the input stream of the event and it must return an empty + * option. + *

+ * If a coercion throws an {@link InvalidFunctionInputException}, the message will be communicated to the user in a + * "bad request" error and no further coercions will be tried. *

- * If a coercion throws a RuntimeException, no further coercions will be tried and the function invocation will fail. + * If a coercion throws any other RuntimeException, no further coercions will be tried and the entire function will + * fail. * * @param currentContext the invocation context for this event - this links to the {@link com.fnproject.fn.api.RuntimeContext} and method and class * @param arg the parameter index for the argument being extracted * @param input the input event - * @return the result of the coercion, if it succeeded + * @return the result of the coercion, if it succeeded */ - Optional tryCoerceParam(InvocationContext currentContext, int arg, InputEvent input); + Optional tryCoerceParam(InvocationContext currentContext, int arg, InputEvent input) throws InvalidFunctionInputException; } diff --git a/runtime/src/main/java/com/fnproject/fn/runtime/EntryPoint.java b/runtime/src/main/java/com/fnproject/fn/runtime/EntryPoint.java index 6493dbca..b6561103 100644 --- a/runtime/src/main/java/com/fnproject/fn/runtime/EntryPoint.java +++ b/runtime/src/main/java/com/fnproject/fn/runtime/EntryPoint.java @@ -2,6 +2,7 @@ import com.fnproject.fn.api.FunctionInvoker; +import com.fnproject.fn.api.InputCoercion; import com.fnproject.fn.api.InputEvent; import com.fnproject.fn.api.OutputEvent; import com.fnproject.fn.runtime.flow.FlowContinuationInvoker; @@ -71,7 +72,7 @@ public int run(Map env, InputStream functionInput, OutputStream } else if (format == null || format.equalsIgnoreCase("default")) { codec = new DefaultEventCodec(env, functionInput, functionOutput); } else { - throw new FunctionInputHandlingException("Unsupported function format:" + format); + throw new FunctionFormatException("Unsupported function format: " + format); } do { @@ -101,6 +102,10 @@ public int run(Map env, InputStream functionInput, OutputStream lastStatus = 1; fic.fireOnFailedInvocation(); } + } catch (InputCoercion.InvalidFunctionInputException e) { + fic.fireOnFailedInvocation(); + lastStatus = 1; + codec.writeEvent(OutputEvent.fromBytes(e.getMessage().getBytes(), 400, "text/plain")); } catch (IOException e) { fic.fireOnFailedInvocation(); throw new FunctionInputHandlingException("Error closing function input", e); diff --git a/runtime/src/main/java/com/fnproject/fn/runtime/MethodFunctionInvoker.java b/runtime/src/main/java/com/fnproject/fn/runtime/MethodFunctionInvoker.java index b0da96cb..12667a5f 100644 --- a/runtime/src/main/java/com/fnproject/fn/runtime/MethodFunctionInvoker.java +++ b/runtime/src/main/java/com/fnproject/fn/runtime/MethodFunctionInvoker.java @@ -1,13 +1,10 @@ package com.fnproject.fn.runtime; -import com.fnproject.fn.api.FunctionInvoker; -import com.fnproject.fn.api.InputEvent; -import com.fnproject.fn.api.InvocationContext; -import com.fnproject.fn.api.OutputEvent; +import com.fnproject.fn.api.*; import com.fnproject.fn.runtime.exception.FunctionInputHandlingException; -import com.fnproject.fn.runtime.exception.InternalFunctionInvocationException; import com.fnproject.fn.runtime.exception.FunctionOutputHandlingException; +import com.fnproject.fn.runtime.exception.InternalFunctionInvocationException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; @@ -29,7 +26,7 @@ public class MethodFunctionInvoker implements FunctionInvoker { * @throws InternalFunctionInvocationException if the invocation fails */ @Override - public Optional tryInvoke(InvocationContext ctx, InputEvent evt) throws InternalFunctionInvocationException { + public Optional tryInvoke(InvocationContext ctx, InputEvent evt) { Object[] userFunctionParams = doInputCoercions(ctx, evt); @@ -59,18 +56,25 @@ private Object[] doInputCoercions(InvocationContext ctx, InputEvent evt) { for (int i = 0; i < userFunctionParams.length; i++) { int param = i; - Optional arg = runtimeContext.getInputCoercions(targetMethod, param) - .stream() - .map((c) -> c.tryCoerceParam(ctx, param, evt)) - .filter(Optional::isPresent) - .map(Optional::get).findFirst(); - - userFunctionParams[i] = arg.orElseThrow(() -> new FunctionInputHandlingException("No type coercion for argument " + param + " of " + targetMethod + " of found")); + Optional arg = Optional.empty(); + for (InputCoercion ic : runtimeContext.getInputCoercions(targetMethod, param)) { + arg = ic.tryCoerceParam(ctx, param, evt); + if (arg.isPresent()) { + break; + } + } + + userFunctionParams[i] = arg.orElseThrow(() -> new InputCoercion.InvalidFunctionInputException( + "No type coercion was able to convert the input provided to the function into a usable form. (param=" + + param + ", method=" + targetMethod + ")")); } return userFunctionParams; - + } catch (InputCoercion.InvalidFunctionInputException e) { + // just rethrow + throw e; } catch (RuntimeException e) { + // Any other exception, we wrap into our own throw new FunctionInputHandlingException("An exception was thrown during Input Coercion: " + e.getMessage(), e); } diff --git a/runtime/src/main/java/com/fnproject/fn/runtime/coercion/ByteArrayCoercion.java b/runtime/src/main/java/com/fnproject/fn/runtime/coercion/ByteArrayCoercion.java index 7908e98c..84c0c0ae 100644 --- a/runtime/src/main/java/com/fnproject/fn/runtime/coercion/ByteArrayCoercion.java +++ b/runtime/src/main/java/com/fnproject/fn/runtime/coercion/ByteArrayCoercion.java @@ -41,7 +41,7 @@ public Optional tryCoerceParam(InvocationContext currentContext, int arg try { return toByteArray(is); } catch (IOException e) { - throw new RuntimeException("Error reading input as bytes", e); + throw new InvalidFunctionInputException("Error reading input as bytes", e); } })); } else { diff --git a/runtime/src/main/java/com/fnproject/fn/runtime/coercion/StringCoercion.java b/runtime/src/main/java/com/fnproject/fn/runtime/coercion/StringCoercion.java index 3c95fda5..cfb46460 100644 --- a/runtime/src/main/java/com/fnproject/fn/runtime/coercion/StringCoercion.java +++ b/runtime/src/main/java/com/fnproject/fn/runtime/coercion/StringCoercion.java @@ -25,7 +25,7 @@ public Optional tryCoerceParam(InvocationContext currentContext, int par try { return IOUtils.toString(is, StandardCharsets.UTF_8); } catch (IOException e) { - throw new RuntimeException("Error reading input as string"); + throw new InvalidFunctionInputException("Error reading input as string"); } })); } else { diff --git a/runtime/src/main/java/com/fnproject/fn/runtime/coercion/jackson/JacksonCoercion.java b/runtime/src/main/java/com/fnproject/fn/runtime/coercion/jackson/JacksonCoercion.java index 84a9e956..f9a82a66 100644 --- a/runtime/src/main/java/com/fnproject/fn/runtime/coercion/jackson/JacksonCoercion.java +++ b/runtime/src/main/java/com/fnproject/fn/runtime/coercion/jackson/JacksonCoercion.java @@ -52,12 +52,12 @@ public Optional tryCoerceParam(InvocationContext currentContext, int par } - private static RuntimeException coercionFailed(Type paramType, Throwable cause) { - return new RuntimeException("Failed to coerce event to user function parameter type " + paramType, cause); + private static InvalidFunctionInputException coercionFailed(Type paramType, Throwable cause) { + return new InvalidFunctionInputException("Failed to coerce function input to user function parameter type " + paramType, cause); } - private static RuntimeException coercionFailed(Type paramType) { + private static InvalidFunctionInputException coercionFailed(Type paramType) { return coercionFailed(paramType, null); } diff --git a/runtime/src/main/java/com/fnproject/fn/runtime/exception/FunctionFormatException.java b/runtime/src/main/java/com/fnproject/fn/runtime/exception/FunctionFormatException.java new file mode 100644 index 00000000..8e2fef4a --- /dev/null +++ b/runtime/src/main/java/com/fnproject/fn/runtime/exception/FunctionFormatException.java @@ -0,0 +1,15 @@ +package com.fnproject.fn.runtime.exception; + +/** + * An exception thrown when the function format is invalid or not supported by the Java FDK, therefore the function + * code cannot be invoked. + */ +public class FunctionFormatException extends FunctionLoadException { + public FunctionFormatException(String message, Throwable cause) { + super(message, cause); + } + + public FunctionFormatException(String s) { + super(s); + } +} diff --git a/runtime/src/test/java/com/fnproject/fn/runtime/ErrorMessagesTest.java b/runtime/src/test/java/com/fnproject/fn/runtime/ErrorMessagesTest.java index 36838e91..b5cc007b 100644 --- a/runtime/src/test/java/com/fnproject/fn/runtime/ErrorMessagesTest.java +++ b/runtime/src/test/java/com/fnproject/fn/runtime/ErrorMessagesTest.java @@ -1,5 +1,6 @@ package com.fnproject.fn.runtime; +import com.fnproject.fn.api.OutputEvent; import com.fnproject.fn.runtime.testfns.ErrorMessages; import not.in.com.fnproject.fn.StacktraceFilteringTestFunctions; import org.junit.Rule; @@ -32,6 +33,13 @@ private void assertIsFunctionErrorWithStacktrace(String errorMessage) { assertThat(fn.getStdErrAsString()).doesNotContain("at com.fnproject.fn.runtime"); } + private void assertIsFunctionErrorWithFailedResponse(String errorMessage) { + assertThat(fn.exitStatus()).isEqualTo(1); + assertThat(fn.getParsedHttpResponses().size()).isEqualTo(1); + assertThat(fn.getParsedHttpResponses().get(0).getStatus()).isGreaterThanOrEqualTo(400); + assertThat(fn.getParsedHttpResponses().get(0).getBodyAsString()).isEqualTo(errorMessage); + } + /* The test method names should be mentally prefixed with "we get a nice error message when..." */ @Test @@ -53,10 +61,17 @@ public void userSpecifiesMethodWhichDoesNotExist(){ } @Test - public void userFunctionInputCoercionError(){ - fn.givenDefaultEvent().withBody("This is not a...").enqueue(); + public void inputIsGenuinelyInvalidOnHotFunction(){ + fn.givenHttpEvent().withBody("This is not a...").enqueue(); + fn.thenRun(ErrorMessages.OtherMethodsClass.class, "takesAnInteger"); + assertIsFunctionErrorWithFailedResponse("Failed to coerce function input to user function parameter type class java.lang.Integer"); + } + + @Test + public void inputIsGenuinelyInvalidOnDefaultFunction(){ + fn.givenHttpEvent().withBody("This is not a...").enqueue(); fn.thenRun(ErrorMessages.OtherMethodsClass.class, "takesAnInteger"); - assertIsEntrypointErrorWithStacktrace("An exception was thrown during Input Coercion: Failed to coerce event to user function parameter type class java.lang.Integer"); + assertIsFunctionErrorWithFailedResponse("Failed to coerce function input to user function parameter type class java.lang.Integer"); } @Test diff --git a/runtime/src/test/java/com/fnproject/fn/runtime/JacksonCoercionTest.java b/runtime/src/test/java/com/fnproject/fn/runtime/JacksonCoercionTest.java index beb04726..845540bf 100644 --- a/runtime/src/test/java/com/fnproject/fn/runtime/JacksonCoercionTest.java +++ b/runtime/src/test/java/com/fnproject/fn/runtime/JacksonCoercionTest.java @@ -57,7 +57,7 @@ public void failureToParseIsUserFriendlyError() throws NoSuchMethodException { causedExpectedError = false; } catch (RuntimeException e) { causedExpectedError = true; - Assert.assertEquals("Failed to coerce event to user function parameter type java.util.List", e.getMessage()); + Assert.assertEquals("Failed to coerce function input to user function parameter type java.util.List", e.getMessage()); Assert.assertTrue(e.getCause().getMessage().startsWith("Unrecognized token 'INVALID':")); } Assert.assertTrue(causedExpectedError);