Skip to content

Commit

Permalink
Throw FlowFrameworkException instead of IOException on parsing errors (
Browse files Browse the repository at this point in the history
…#509)

Signed-off-by: Daniel Widdis <[email protected]>
  • Loading branch information
dbwiddis committed Feb 9, 2024
1 parent 13b1f93 commit 33ff2f3
Show file tree
Hide file tree
Showing 14 changed files with 99 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@
*/
package org.opensearch.flowframework.model;

import org.opensearch.core.rest.RestStatus;
import org.opensearch.core.xcontent.ToXContentObject;
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.core.xcontent.XContentParser;
import org.opensearch.flowframework.exception.FlowFrameworkException;

import java.io.IOException;
import java.util.HashMap;
Expand Down Expand Up @@ -75,11 +77,14 @@ public static PipelineProcessor parse(XContentParser parser) throws IOException
params = parseStringToStringMap(parser);
break;
default:
throw new IOException("Unable to parse field [" + fieldName + "] in a pipeline processor object.");
throw new FlowFrameworkException(
"Unable to parse field [" + fieldName + "] in a pipeline processor object.",
RestStatus.BAD_REQUEST
);
}
}
if (type == null) {
throw new IOException("A processor object requires a type field.");
throw new FlowFrameworkException("A processor object requires a type field.", RestStatus.BAD_REQUEST);
}

return new PipelineProcessor(type, params);
Expand Down
14 changes: 11 additions & 3 deletions src/main/java/org/opensearch/flowframework/model/Template.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@
import org.opensearch.common.xcontent.json.JsonXContent;
import org.opensearch.common.xcontent.yaml.YamlXContent;
import org.opensearch.commons.authuser.User;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.core.xcontent.ToXContentObject;
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.core.xcontent.XContentParser;
import org.opensearch.flowframework.exception.FlowFrameworkException;

import java.io.IOException;
import java.util.ArrayList;
Expand Down Expand Up @@ -292,7 +294,10 @@ public static Template parse(XContentParser parser) throws IOException {
}
break;
default:
throw new IOException("Unable to parse field [" + fieldName + "] in a version object.");
throw new FlowFrameworkException(
"Unable to parse field [" + fieldName + "] in a version object.",
RestStatus.BAD_REQUEST
);
}
}
break;
Expand All @@ -311,11 +316,14 @@ public static Template parse(XContentParser parser) throws IOException {
user = User.parse(parser);
break;
default:
throw new IOException("Unable to parse field [" + fieldName + "] in a template object.");
throw new FlowFrameworkException(
"Unable to parse field [" + fieldName + "] in a template object.",
RestStatus.BAD_REQUEST
);
}
}
if (name == null) {
throw new IOException("A template object requires a name.");
throw new FlowFrameworkException("A template object requires a name.", RestStatus.BAD_REQUEST);
}

return new Builder().name(name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@
*/
package org.opensearch.flowframework.model;

import org.opensearch.core.rest.RestStatus;
import org.opensearch.core.xcontent.ToXContentObject;
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.core.xcontent.XContentParser;
import org.opensearch.flowframework.exception.FlowFrameworkException;
import org.opensearch.flowframework.workflow.WorkflowData;

import java.io.IOException;
Expand Down Expand Up @@ -118,7 +120,7 @@ public static Workflow parse(XContentParser parser) throws IOException {

}
if (nodes.isEmpty()) {
throw new IOException("A workflow must have at least one node.");
throw new FlowFrameworkException("A workflow must have at least one node.", RestStatus.BAD_REQUEST);
}
// Iterate the nodes and infer edges from previous node inputs
List<WorkflowEdge> inferredEdges = nodes.stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@
*/
package org.opensearch.flowframework.model;

import org.opensearch.core.rest.RestStatus;
import org.opensearch.core.xcontent.ToXContentObject;
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.core.xcontent.XContentParser;
import org.opensearch.flowframework.exception.FlowFrameworkException;

import java.io.IOException;
import java.util.Objects;
Expand Down Expand Up @@ -72,11 +74,14 @@ public static WorkflowEdge parse(XContentParser parser) throws IOException {
destination = parser.text();
break;
default:
throw new IOException("Unable to parse field [" + fieldName + "] in an edge object.");
throw new FlowFrameworkException(
"Unable to parse field [" + fieldName + "] in an edge object.",
RestStatus.BAD_REQUEST
);
}
}
if (source == null || destination == null) {
throw new IOException("An edge object requires both a source and dest field.");
throw new FlowFrameworkException("An edge object requires both a source and dest field.", RestStatus.BAD_REQUEST);
}

return new WorkflowEdge(source, destination);
Expand Down
16 changes: 12 additions & 4 deletions src/main/java/org/opensearch/flowframework/model/WorkflowNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@
package org.opensearch.flowframework.model;

import org.opensearch.common.unit.TimeValue;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.core.xcontent.ToXContentObject;
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.core.xcontent.XContentParser;
import org.opensearch.flowframework.exception.FlowFrameworkException;
import org.opensearch.flowframework.workflow.ProcessNode;
import org.opensearch.flowframework.workflow.WorkflowData;
import org.opensearch.flowframework.workflow.WorkflowStep;
Expand Down Expand Up @@ -190,23 +192,29 @@ public static WorkflowNode parse(XContentParser parser) throws IOException {
userInputs.put(inputFieldName, parser.bigIntegerValue());
break;
default:
throw new IOException("Unable to parse field [" + inputFieldName + "] in a node object.");
throw new FlowFrameworkException(
"Unable to parse field [" + inputFieldName + "] in a node object.",
RestStatus.BAD_REQUEST
);
}
break;
case VALUE_BOOLEAN:
userInputs.put(inputFieldName, parser.booleanValue());
break;
default:
throw new IOException("Unable to parse field [" + inputFieldName + "] in a node object.");
throw new FlowFrameworkException(
"Unable to parse field [" + inputFieldName + "] in a node object.",
RestStatus.BAD_REQUEST
);
}
}
break;
default:
throw new IOException("Unable to parse field [" + fieldName + "] in a node object.");
throw new FlowFrameworkException("Unable to parse field [" + fieldName + "] in a node object.", RestStatus.BAD_REQUEST);
}
}
if (id == null || type == null) {
throw new IOException("An node object requires both an id and type field.");
throw new FlowFrameworkException("An node object requires both an id and type field.", RestStatus.BAD_REQUEST);
}

return new WorkflowNode(id, type, previousNodeInputs, userInputs);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,10 @@ public static WorkflowState parse(XContentParser parser) throws IOException {
userOutputs.put(userOutputsFieldName, parseStringToStringMap(parser));
break;
default:
throw new IOException("Unable to parse field [" + userOutputsFieldName + "] in a user_outputs object.");
throw new FlowFrameworkException(
"Unable to parse field [" + userOutputsFieldName + "] in a user_outputs object.",
RestStatus.BAD_REQUEST
);
}
}
break;
Expand All @@ -390,7 +393,10 @@ public static WorkflowState parse(XContentParser parser) throws IOException {
}
break;
default:
throw new IOException("Unable to parse field [" + fieldName + "] in a workflowState object.");
throw new FlowFrameworkException(
"Unable to parse field [" + fieldName + "] in a workflowState object.",
RestStatus.BAD_REQUEST
);
}
}
return new Builder().workflowId(workflowId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,10 @@ public static WorkflowStepValidator parse(XContentParser parser) throws IOExcept
}
break;
default:
throw new IOException("Unable to parse field [" + fieldName + "] in a WorkflowStepValidator object.");
throw new FlowFrameworkException(
"Unable to parse field [" + fieldName + "] in a WorkflowStepValidator object.",
RestStatus.BAD_REQUEST
);
}
}
return new WorkflowStepValidator(parsedInputs, parsedOutputs, requiredPlugins, timeout);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
*/
package org.opensearch.flowframework.model;

import org.opensearch.core.rest.RestStatus;
import org.opensearch.flowframework.exception.FlowFrameworkException;
import org.opensearch.test.OpenSearchTestCase;

import java.io.IOException;
Expand All @@ -32,12 +34,17 @@ public void testProcessor() throws IOException {

public void testExceptions() throws IOException {
String badJson = "{\"badField\":\"foo\",\"params\":{\"bar\":\"baz\"}}";
IOException e = assertThrows(IOException.class, () -> PipelineProcessor.parse(TemplateTestJsonUtil.jsonToParser(badJson)));
FlowFrameworkException e = assertThrows(
FlowFrameworkException.class,
() -> PipelineProcessor.parse(TemplateTestJsonUtil.jsonToParser(badJson))
);
assertEquals("Unable to parse field [badField] in a pipeline processor object.", e.getMessage());
assertEquals(RestStatus.BAD_REQUEST, e.getRestStatus());

String noTypeJson = "{\"params\":{\"bar\":\"baz\"}}";
e = assertThrows(IOException.class, () -> PipelineProcessor.parse(TemplateTestJsonUtil.jsonToParser(noTypeJson)));
e = assertThrows(FlowFrameworkException.class, () -> PipelineProcessor.parse(TemplateTestJsonUtil.jsonToParser(noTypeJson)));
assertEquals("A processor object requires a type field.", e.getMessage());
assertEquals(RestStatus.BAD_REQUEST, e.getRestStatus());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
package org.opensearch.flowframework.model;

import org.opensearch.Version;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.flowframework.exception.FlowFrameworkException;
import org.opensearch.test.OpenSearchTestCase;

import java.io.IOException;
Expand Down Expand Up @@ -74,15 +76,17 @@ public void testTemplate() throws IOException {
}

public void testExceptions() throws IOException {
IOException e;
FlowFrameworkException e;

String badTemplateField = expectedTemplate.replace("use_case", "badField");
e = assertThrows(IOException.class, () -> Template.parse(badTemplateField));
e = assertThrows(FlowFrameworkException.class, () -> Template.parse(badTemplateField));
assertEquals("Unable to parse field [badField] in a template object.", e.getMessage());
assertEquals(RestStatus.BAD_REQUEST, e.getRestStatus());

String badVersionField = expectedTemplate.replace("compatibility", "badField");
e = assertThrows(IOException.class, () -> Template.parse(badVersionField));
e = assertThrows(FlowFrameworkException.class, () -> Template.parse(badVersionField));
assertEquals("Unable to parse field [version] in a version object.", e.getMessage());
assertEquals(RestStatus.BAD_REQUEST, e.getRestStatus());
}

public void testStrings() throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
*/
package org.opensearch.flowframework.model;

import org.opensearch.core.rest.RestStatus;
import org.opensearch.flowframework.exception.FlowFrameworkException;
import org.opensearch.test.OpenSearchTestCase;

import java.io.IOException;
Expand Down Expand Up @@ -43,12 +45,17 @@ public void testEdge() throws IOException {

public void testExceptions() throws IOException {
String badJson = "{\"badField\":\"A\",\"dest\":\"B\"}";
IOException e = assertThrows(IOException.class, () -> WorkflowEdge.parse(TemplateTestJsonUtil.jsonToParser(badJson)));
FlowFrameworkException e = assertThrows(
FlowFrameworkException.class,
() -> WorkflowEdge.parse(TemplateTestJsonUtil.jsonToParser(badJson))
);
assertEquals("Unable to parse field [badField] in an edge object.", e.getMessage());
assertEquals(RestStatus.BAD_REQUEST, e.getRestStatus());

String missingJson = "{\"dest\":\"B\"}";
e = assertThrows(IOException.class, () -> WorkflowEdge.parse(TemplateTestJsonUtil.jsonToParser(missingJson)));
e = assertThrows(FlowFrameworkException.class, () -> WorkflowEdge.parse(TemplateTestJsonUtil.jsonToParser(missingJson)));
assertEquals("An edge object requires both a source and dest field.", e.getMessage());
assertEquals(RestStatus.BAD_REQUEST, e.getRestStatus());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
*/
package org.opensearch.flowframework.model;

import org.opensearch.core.rest.RestStatus;
import org.opensearch.flowframework.exception.FlowFrameworkException;
import org.opensearch.test.OpenSearchTestCase;

import java.io.IOException;
Expand Down Expand Up @@ -87,11 +89,16 @@ public void testNode() throws IOException {

public void testExceptions() throws IOException {
String badJson = "{\"badField\":\"A\",\"type\":\"a-type\",\"user_inputs\":{\"foo\":\"bar\"}}";
IOException e = assertThrows(IOException.class, () -> WorkflowNode.parse(TemplateTestJsonUtil.jsonToParser(badJson)));
FlowFrameworkException e = assertThrows(
FlowFrameworkException.class,
() -> WorkflowNode.parse(TemplateTestJsonUtil.jsonToParser(badJson))
);
assertEquals("Unable to parse field [badField] in a node object.", e.getMessage());
assertEquals(RestStatus.BAD_REQUEST, e.getRestStatus());

String missingJson = "{\"id\":\"A\",\"user_inputs\":{\"foo\":\"bar\"}}";
e = assertThrows(IOException.class, () -> WorkflowNode.parse(TemplateTestJsonUtil.jsonToParser(missingJson)));
e = assertThrows(FlowFrameworkException.class, () -> WorkflowNode.parse(TemplateTestJsonUtil.jsonToParser(missingJson)));
assertEquals("An node object requires both an id and type field.", e.getMessage());
assertEquals(RestStatus.BAD_REQUEST, e.getRestStatus());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
*/
package org.opensearch.flowframework.model;

import org.opensearch.core.rest.RestStatus;
import org.opensearch.core.xcontent.XContentParser;
import org.opensearch.flowframework.exception.FlowFrameworkException;
import org.opensearch.test.OpenSearchTestCase;

import java.io.IOException;
Expand Down Expand Up @@ -38,9 +40,9 @@ public void testParseWorkflowStepValidator() throws IOException {

public void testFailedParseWorkflowStepValidator() throws IOException {
XContentParser parser = TemplateTestJsonUtil.jsonToParser(invalidValidator);
IOException ex = expectThrows(IOException.class, () -> WorkflowStepValidator.parse(parser));
FlowFrameworkException ex = expectThrows(FlowFrameworkException.class, () -> WorkflowStepValidator.parse(parser));
assertEquals("Unable to parse field [invalid_field] in a WorkflowStepValidator object.", ex.getMessage());

assertEquals(RestStatus.BAD_REQUEST, ex.getRestStatus());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@
import org.opensearch.common.settings.ClusterSettings;
import org.opensearch.common.settings.Setting;
import org.opensearch.common.settings.Settings;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.core.xcontent.XContentParser;
import org.opensearch.flowframework.common.FlowFrameworkSettings;
import org.opensearch.flowframework.exception.FlowFrameworkException;
import org.opensearch.flowframework.indices.FlowFrameworkIndicesHandler;
import org.opensearch.flowframework.workflow.WorkflowStepFactory;
import org.opensearch.ml.client.MachineLearningNodeClient;
Expand Down Expand Up @@ -71,8 +73,9 @@ public void testParseWorkflowValidator() throws IOException {

public void testFailedParseWorkflowValidator() throws IOException {
XContentParser parser = TemplateTestJsonUtil.jsonToParser(invalidWorkflowStepJson);
IOException ex = expectThrows(IOException.class, () -> WorkflowValidator.parse(parser));
FlowFrameworkException ex = expectThrows(FlowFrameworkException.class, () -> WorkflowValidator.parse(parser));
assertEquals("Unable to parse field [bad_field] in a WorkflowStepValidator object.", ex.getMessage());
assertEquals(RestStatus.BAD_REQUEST, ex.getRestStatus());
}

public void testWorkflowStepFactoryHasValidators() throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,12 @@ public void testCycles() {

public void testNoEdges() throws IOException {
List<String> workflow;
Exception ex = assertThrows(IOException.class, () -> parse(workflow(Collections.emptyList(), Collections.emptyList())));
FlowFrameworkException ex = assertThrows(
FlowFrameworkException.class,
() -> parse(workflow(Collections.emptyList(), Collections.emptyList()))
);
assertEquals(MUST_HAVE_AT_LEAST_ONE_NODE, ex.getMessage());
assertEquals(RestStatus.BAD_REQUEST, ex.getRestStatus());

workflow = parse(workflow(List.of(node("A")), Collections.emptyList()));
assertEquals(1, workflow.size());
Expand Down

0 comments on commit 33ff2f3

Please sign in to comment.