Skip to content

Commit 0ab5778

Browse files
opensearch-trigger-bot[bot]pyek-botdhrubo-os
authored
[ExceptionHandling] Throw proper 400 errors instead of 500 for agent execute and MCP (#3988) (#4051)
* improve exception handling * feat: add test case for jsonparseexception --------- (cherry picked from commit 943ef8c) Signed-off-by: Pavan Yekbote <pybot@amazon.com> Co-authored-by: Pavan Yekbote <pybot@amazon.com> Co-authored-by: Dhrubo Saha <dhrubo@amazon.com>
1 parent eae6ca5 commit 0ab5778

File tree

3 files changed

+29
-5
lines changed

3 files changed

+29
-5
lines changed

common/src/main/java/org/opensearch/ml/common/MLCommonsClassLoader.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
import org.opensearch.ml.common.output.MLOutputType;
2929
import org.reflections.Reflections;
3030

31+
import com.fasterxml.jackson.core.JsonParseException;
32+
3133
import lombok.extern.log4j.Log4j2;
3234

3335
@Log4j2
@@ -255,16 +257,18 @@ public static boolean canInitMLInput(FunctionName functionName) {
255257
return mlInputClassMap.containsKey(functionName);
256258
}
257259

258-
public static <S> S initConnector(String name, Object[] initArgs, Class<?>... constructorParameterTypes) {
260+
public static <S> S initConnector(String name, Object[] initArgs, Class<?>... constructorParameterTypes) throws JsonParseException {
259261
return init(connectorClassMap, name, initArgs, constructorParameterTypes);
260262
}
261263

262264
@SuppressWarnings("unchecked")
263-
public static <T extends Enum<T>, S> S initMLInput(T type, Object[] initArgs, Class<?>... constructorParameterTypes) {
265+
public static <T extends Enum<T>, S> S initMLInput(T type, Object[] initArgs, Class<?>... constructorParameterTypes)
266+
throws JsonParseException {
264267
return init(mlInputClassMap, type, initArgs, constructorParameterTypes);
265268
}
266269

267-
private static <T, S> S init(Map<T, Class<?>> map, T type, Object[] initArgs, Class<?>... constructorParameterTypes) {
270+
private static <T, S> S init(Map<T, Class<?>> map, T type, Object[] initArgs, Class<?>... constructorParameterTypes)
271+
throws JsonParseException {
268272
Class<?> clazz = map.get(type);
269273
if (clazz == null) {
270274
throw new IllegalArgumentException("Can't find class for type " + type);
@@ -278,6 +282,8 @@ private static <T, S> S init(Map<T, Class<?>> map, T type, Object[] initArgs, Cl
278282
throw (MLException) cause;
279283
} else if (cause instanceof IllegalArgumentException) {
280284
throw (IllegalArgumentException) cause;
285+
} else if (cause instanceof JsonParseException) {
286+
throw (JsonParseException) cause;
281287
} else {
282288
log.error("Failed to init instance for type " + type, e);
283289
return null;

common/src/test/java/org/opensearch/ml/common/MLCommonsClassLoaderTests.java

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@
4040
import org.opensearch.ml.common.output.execute.samplecalculator.LocalSampleCalculatorOutput;
4141
import org.opensearch.search.SearchModule;
4242

43+
import com.fasterxml.jackson.core.JsonParseException;
44+
4345
public class MLCommonsClassLoaderTests {
4446

4547
private SampleAlgoParams params;
@@ -183,14 +185,30 @@ public void testClassLoader_MLInput() throws IOException {
183185
}
184186

185187
@Test(expected = IllegalArgumentException.class)
186-
public void testConnectorInitializationException() {
188+
public void testConnectorInitializationException() throws JsonParseException {
187189
// Example initialization parameters for connectors
188190
String initParam1 = "parameter1";
189191

190192
// Initialize the first connector type
191193
MLCommonsClassLoader.initConnector("Connector", new Object[] { initParam1 }, String.class);
192194
}
193195

196+
@Test(expected = JsonParseException.class)
197+
public void testInitMLInput_JsonParseException() throws IOException {
198+
String invalidJsonStr = "invalid-json";
199+
XContentParser parser = XContentType.JSON
200+
.xContent()
201+
.createParser(
202+
new NamedXContentRegistry(new SearchModule(Settings.EMPTY, Collections.emptyList()).getNamedXContents()),
203+
null,
204+
invalidJsonStr
205+
);
206+
parser.nextToken();
207+
208+
MLCommonsClassLoader
209+
.initMLInput(FunctionName.AGENT, new Object[] { parser, FunctionName.AGENT }, XContentParser.class, FunctionName.class);
210+
}
211+
194212
public enum TestEnum {
195213
TEST
196214
}

plugin/src/main/java/org/opensearch/ml/action/mcpserver/TransportMcpToolsRegisterAction.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ protected void doExecute(Task task, ActionRequest request, ActionListener<MLMcpT
115115
String exceptionMessage = String
116116
.format(Locale.ROOT, "Unable to register tools: %s as they already exist", existingTools);
117117
log.warn(exceptionMessage);
118-
restoreListener.onFailure(new OpenSearchException(exceptionMessage));
118+
restoreListener.onFailure(new IllegalArgumentException(exceptionMessage));
119119
} else {
120120
indexMcpTools(registerNodesRequest, restoreListener);
121121
}

0 commit comments

Comments
 (0)