Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions api/src/main/java/com/fnproject/fn/api/FunctionInvoker.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
* <p>
* 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.
* <p>
* An {@link InputCoercion.InvalidFunctionInputException} thrown by the implementation will indicate that the input
* provided to the function by the user is invalid.
* <p>
* 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
Expand Down
29 changes: 24 additions & 5 deletions api/src/main/java/com/fnproject/fn/api/InputCoercion.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,39 @@
* Handles the coercion of an input event to a parameter
*/
public interface InputCoercion<T> {
/**
* 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
* <p>
* 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)
* <p>
* 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.
* <p>
* 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.
* <p>
* 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<T> tryCoerceParam(InvocationContext currentContext, int arg, InputEvent input);
Optional<T> tryCoerceParam(InvocationContext currentContext, int arg, InputEvent input) throws InvalidFunctionInputException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -71,7 +72,7 @@ public int run(Map<String, String> 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 {
Expand Down Expand Up @@ -101,6 +102,10 @@ public int run(Map<String, String> 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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -29,7 +26,7 @@ public class MethodFunctionInvoker implements FunctionInvoker {
* @throws InternalFunctionInvocationException if the invocation fails
*/
@Override
public Optional<OutputEvent> tryInvoke(InvocationContext ctx, InputEvent evt) throws InternalFunctionInvocationException {
public Optional<OutputEvent> tryInvoke(InvocationContext ctx, InputEvent evt) {

Object[] userFunctionParams = doInputCoercions(ctx, evt);

Expand Down Expand Up @@ -59,18 +56,25 @@ private Object[] doInputCoercions(InvocationContext ctx, InputEvent evt) {

for (int i = 0; i < userFunctionParams.length; i++) {
int param = i;
Optional<Object> arg = runtimeContext.getInputCoercions(targetMethod, param)
.stream()
.map((c) -> c.tryCoerceParam(ctx, param, evt))
.filter(Optional::isPresent)
.map(Optional::get).findFirst();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stream usage looks nice but it was actually broken because findFirst() can return null and the subsequent arg.orElseThrow would break. A for loop is also more readable.


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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public Optional<byte[]> 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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public Optional<String> 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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ public Optional<Object> 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);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<com.fnproject.fn.runtime.testfns.Animal>", e.getMessage());
Assert.assertEquals("Failed to coerce function input to user function parameter type java.util.List<com.fnproject.fn.runtime.testfns.Animal>", e.getMessage());
Assert.assertTrue(e.getCause().getMessage().startsWith("Unrecognized token 'INVALID':"));
}
Assert.assertTrue(causedExpectedError);
Expand Down