Skip to content

Return null rather than throwing exception when conversion fail #1222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fjtirado merged 1 commit into from
Mar 10, 2026
Merged

Return null rather than throwing exception when conversion fail #1222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 15 additions & 3 deletions ...mbda/src/main/java/io/serverlessworkflow/impl/expressions/func/JavaExpressionFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import io.serverlessworkflow.api.types.func.TypedJavaContextFunction;
import io.serverlessworkflow.api.types.func.TypedJavaFilterFunction;
import io.serverlessworkflow.api.types.func.TypedPredicate;
import io.serverlessworkflow.impl.WorkflowModel;
import io.serverlessworkflow.impl.WorkflowPredicate;
import io.serverlessworkflow.impl.expressions.AbstractExpressionFactory;
import io.serverlessworkflow.impl.expressions.ExpressionDescriptor;
Expand All @@ -44,20 +45,31 @@ public ObjectExpression buildExpression(ExpressionDescriptor descriptor) {
if (value instanceof Function func) {
return (w, t, n) -> func.apply(n.asJavaObject());
} else if (value instanceof TypedFunction func) {
return (w, t, n) -> func.function().apply(n.as(func.argClass()).orElseThrow());
return (w, t, n) -> func.function().apply(convert(n, func.argClass()));
} else if (value instanceof JavaFilterFunction func) {
return (w, t, n) -> func.apply(n.asJavaObject(), w, t);
} else if (value instanceof TypedJavaFilterFunction func) {
return (w, t, n) -> func.function().apply(n.as(func.argClass()).orElseThrow(), w, t);
return (w, t, n) -> func.function().apply(convert(n, func.argClass()), w, t);
} else if (value instanceof JavaContextFunction func) {
return (w, t, n) -> func.apply(n.asJavaObject(), w);
} else if (value instanceof TypedJavaContextFunction func) {
return (w, t, n) -> func.function().apply(n.as(func.argClass()).orElseThrow(), w);
return (w, t, n) -> func.function().apply(convert(n, func.argClass()), w);
} else {
return (w, t, n) -> value;
}
}

private <T> T convert(WorkflowModel model, Class<T> argClass) {
return model.isNull()
? null
: model
.as(argClass)
.orElseThrow(
() ->
new IllegalArgumentException(
Comment on lines +66 to +69
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

The new model.isNull() ? null : ... conversion path isn’t covered by tests. Adding a test that passes a null workflow/task output into a typed Java function and asserts the function receives null (and no exception is thrown) would prevent regressions.

Copilot uses AI. Check for mistakes.
"Cannot convert model " + model.asJavaObject() + " to class" + argClass));
}

@Override
public int priority(ExpressionDescriptor descriptor) {
Comment on lines +71 to 74
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

The IllegalArgumentException message concatenation is missing a space before the target type (currently produces ... to classclass ...). Consider formatting it as ... to class <type> (e.g., using argClass.getName()), and include a separator after class.

Copilot uses AI. Check for mistakes.
Object value = descriptor.asObject();
Comment on lines +66 to 75
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

PR title indicates conversions should return null rather than throw when conversion fails, but this still throws for non-null values when model.as(argClass) is empty (and model.as(...) may also throw). If the intent is truly "return null on conversion failure", consider returning null/Optional here instead of throwing, or explicitly adjust the PR title/behavior to match.

Copilot uses AI. Check for mistakes.
Expand Down
3 changes: 2 additions & 1 deletion .../io/serverless/workflow/impl/executors/func/FuncDSLDataFlowTransformationHelpersTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,8 @@ void test_output_with_exportAs() {
Long taskOutput = output(taskContextData, Long.class);
softly.assertThat(taskOutput).isEqualTo(15L);
return taskOutput * 2;
}))
},
Object.class))
.build();

try (WorkflowApplication app = WorkflowApplication.builder().build()) {
Expand Down
4 changes: 4 additions & 0 deletions impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,8 @@ public interface WorkflowModel {
Class<?> objectClass();

<T> Optional<T> as(Class<T> clazz);

default boolean isNull() {
return asJavaObject() == null;
}
}
5 changes: 5 additions & 0 deletions impl/model/src/main/java/io/serverlessworkflow/impl/model/jackson/JacksonModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ protected JacksonModel(JsonNode node) {
this.node = node;
}

@Override
public boolean isNull() {
return node.isNull();
}

@Override
public Optional<Boolean> asBoolean() {
return node.isBoolean() ? Optional.of(node.asBoolean()) : Optional.empty();
Expand Down