From cb2f454d21f2fc2f444464402310e87eb420bfef Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Fri, 9 Feb 2024 11:41:45 -0800 Subject: [PATCH] [Backport 2.12] Throw FlowFrameworkException instead of IOException on parsing errors (#512) Throw FlowFrameworkException instead of IOException on parsing errors (#509) (cherry picked from commit 33ff2f3dccc2abbf6e770dba913dbc823af7e5ef) Signed-off-by: Daniel Widdis Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] --- .../flowframework/model/PipelineProcessor.java | 9 +++++++-- .../opensearch/flowframework/model/Template.java | 14 +++++++++++--- .../opensearch/flowframework/model/Workflow.java | 4 +++- .../flowframework/model/WorkflowEdge.java | 9 +++++++-- .../flowframework/model/WorkflowNode.java | 16 ++++++++++++---- .../flowframework/model/WorkflowState.java | 10 ++++++++-- .../model/WorkflowStepValidator.java | 5 ++++- .../model/PipelineProcessorTests.java | 11 +++++++++-- .../flowframework/model/TemplateTests.java | 10 +++++++--- .../flowframework/model/WorkflowEdgeTests.java | 11 +++++++++-- .../flowframework/model/WorkflowNodeTests.java | 11 +++++++++-- .../model/WorkflowStepValidatorTests.java | 6 ++++-- .../model/WorkflowValidatorTests.java | 5 ++++- .../workflow/WorkflowProcessSorterTests.java | 6 +++++- 14 files changed, 99 insertions(+), 28 deletions(-) diff --git a/src/main/java/org/opensearch/flowframework/model/PipelineProcessor.java b/src/main/java/org/opensearch/flowframework/model/PipelineProcessor.java index f4f6f7d4e..83e18d128 100644 --- a/src/main/java/org/opensearch/flowframework/model/PipelineProcessor.java +++ b/src/main/java/org/opensearch/flowframework/model/PipelineProcessor.java @@ -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; @@ -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); diff --git a/src/main/java/org/opensearch/flowframework/model/Template.java b/src/main/java/org/opensearch/flowframework/model/Template.java index 0c081b0d2..0b6e885f0 100644 --- a/src/main/java/org/opensearch/flowframework/model/Template.java +++ b/src/main/java/org/opensearch/flowframework/model/Template.java @@ -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; @@ -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; @@ -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) diff --git a/src/main/java/org/opensearch/flowframework/model/Workflow.java b/src/main/java/org/opensearch/flowframework/model/Workflow.java index a1fd4f4b5..f60262f3a 100644 --- a/src/main/java/org/opensearch/flowframework/model/Workflow.java +++ b/src/main/java/org/opensearch/flowframework/model/Workflow.java @@ -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; @@ -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 inferredEdges = nodes.stream() diff --git a/src/main/java/org/opensearch/flowframework/model/WorkflowEdge.java b/src/main/java/org/opensearch/flowframework/model/WorkflowEdge.java index 7fbdaf568..42f894bfe 100644 --- a/src/main/java/org/opensearch/flowframework/model/WorkflowEdge.java +++ b/src/main/java/org/opensearch/flowframework/model/WorkflowEdge.java @@ -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; @@ -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); diff --git a/src/main/java/org/opensearch/flowframework/model/WorkflowNode.java b/src/main/java/org/opensearch/flowframework/model/WorkflowNode.java index bdd889310..55122f9e5 100644 --- a/src/main/java/org/opensearch/flowframework/model/WorkflowNode.java +++ b/src/main/java/org/opensearch/flowframework/model/WorkflowNode.java @@ -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; @@ -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); diff --git a/src/main/java/org/opensearch/flowframework/model/WorkflowState.java b/src/main/java/org/opensearch/flowframework/model/WorkflowState.java index ca1fc8855..29d808fb7 100644 --- a/src/main/java/org/opensearch/flowframework/model/WorkflowState.java +++ b/src/main/java/org/opensearch/flowframework/model/WorkflowState.java @@ -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; @@ -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) diff --git a/src/main/java/org/opensearch/flowframework/model/WorkflowStepValidator.java b/src/main/java/org/opensearch/flowframework/model/WorkflowStepValidator.java index 4673020dd..6f1b4242a 100644 --- a/src/main/java/org/opensearch/flowframework/model/WorkflowStepValidator.java +++ b/src/main/java/org/opensearch/flowframework/model/WorkflowStepValidator.java @@ -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); diff --git a/src/test/java/org/opensearch/flowframework/model/PipelineProcessorTests.java b/src/test/java/org/opensearch/flowframework/model/PipelineProcessorTests.java index 5e9a81d0d..77107f94a 100644 --- a/src/test/java/org/opensearch/flowframework/model/PipelineProcessorTests.java +++ b/src/test/java/org/opensearch/flowframework/model/PipelineProcessorTests.java @@ -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; @@ -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()); } } diff --git a/src/test/java/org/opensearch/flowframework/model/TemplateTests.java b/src/test/java/org/opensearch/flowframework/model/TemplateTests.java index bb01a6b3a..e0b88d725 100644 --- a/src/test/java/org/opensearch/flowframework/model/TemplateTests.java +++ b/src/test/java/org/opensearch/flowframework/model/TemplateTests.java @@ -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; @@ -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 { diff --git a/src/test/java/org/opensearch/flowframework/model/WorkflowEdgeTests.java b/src/test/java/org/opensearch/flowframework/model/WorkflowEdgeTests.java index ffbd07bd1..12cfb4f08 100644 --- a/src/test/java/org/opensearch/flowframework/model/WorkflowEdgeTests.java +++ b/src/test/java/org/opensearch/flowframework/model/WorkflowEdgeTests.java @@ -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; @@ -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()); } } diff --git a/src/test/java/org/opensearch/flowframework/model/WorkflowNodeTests.java b/src/test/java/org/opensearch/flowframework/model/WorkflowNodeTests.java index 4de71834b..41dbe5236 100644 --- a/src/test/java/org/opensearch/flowframework/model/WorkflowNodeTests.java +++ b/src/test/java/org/opensearch/flowframework/model/WorkflowNodeTests.java @@ -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; @@ -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()); } } diff --git a/src/test/java/org/opensearch/flowframework/model/WorkflowStepValidatorTests.java b/src/test/java/org/opensearch/flowframework/model/WorkflowStepValidatorTests.java index 646e8f8af..1426ebfae 100644 --- a/src/test/java/org/opensearch/flowframework/model/WorkflowStepValidatorTests.java +++ b/src/test/java/org/opensearch/flowframework/model/WorkflowStepValidatorTests.java @@ -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; @@ -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()); } } diff --git a/src/test/java/org/opensearch/flowframework/model/WorkflowValidatorTests.java b/src/test/java/org/opensearch/flowframework/model/WorkflowValidatorTests.java index 1b085ae1e..46f1e7950 100644 --- a/src/test/java/org/opensearch/flowframework/model/WorkflowValidatorTests.java +++ b/src/test/java/org/opensearch/flowframework/model/WorkflowValidatorTests.java @@ -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; @@ -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 { diff --git a/src/test/java/org/opensearch/flowframework/workflow/WorkflowProcessSorterTests.java b/src/test/java/org/opensearch/flowframework/workflow/WorkflowProcessSorterTests.java index 8825b0815..d66e4f391 100644 --- a/src/test/java/org/opensearch/flowframework/workflow/WorkflowProcessSorterTests.java +++ b/src/test/java/org/opensearch/flowframework/workflow/WorkflowProcessSorterTests.java @@ -242,8 +242,12 @@ public void testCycles() { public void testNoEdges() throws IOException { List 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());