From b41394706935c46a1422ca338639c40118f01734 Mon Sep 17 00:00:00 2001 From: Christian Tzolov Date: Tue, 16 Sep 2025 10:10:22 +0200 Subject: [PATCH 1/3] refactor: consolidate MCP tool callback hierarchy and improve error handling - Introduce AbstractMcpToolMethodCallback as common base class to eliminate code duplication - Rename error handling methods for clarity: - createErrorResult -> createAsyncErrorResult/createSyncErrorResult - validateRequest -> validateSyncRequest - Improve error messages using findCauseUsingPlainJava(e) to extract and display root cause instead of wrapper exceptions Signed-off-by: Christian Tzolov --- .../AbstractAsyncMcpToolMethodCallback.java | 149 +------------ .../tool/AbstractMcpToolMethodCallback.java | 206 ++++++++++++++++++ .../AbstractSyncMcpToolMethodCallback.java | 174 ++------------- .../tool/AsyncMcpToolMethodCallback.java | 2 +- .../AsyncStatelessMcpToolMethodCallback.java | 2 +- .../tool/SyncMcpToolMethodCallback.java | 4 +- .../SyncStatelessMcpToolMethodCallback.java | 4 +- ...lMethodCallbackExceptionHandlingTests.java | 15 +- .../tool/SyncMcpToolMethodCallbackTests.java | 8 +- ...ncStatelessMcpToolMethodCallbackTests.java | 6 +- 10 files changed, 252 insertions(+), 318 deletions(-) create mode 100644 mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AbstractMcpToolMethodCallback.java diff --git a/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AbstractAsyncMcpToolMethodCallback.java b/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AbstractAsyncMcpToolMethodCallback.java index 5ac28ce..c73554e 100644 --- a/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AbstractAsyncMcpToolMethodCallback.java +++ b/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AbstractAsyncMcpToolMethodCallback.java @@ -16,22 +16,13 @@ package org.springaicommunity.mcp.method.tool; -import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.lang.reflect.Type; -import java.util.Map; -import java.util.stream.Stream; +import io.modelcontextprotocol.spec.McpSchema.CallToolRequest; +import io.modelcontextprotocol.spec.McpSchema.CallToolResult; import org.reactivestreams.Publisher; -import org.springaicommunity.mcp.annotation.McpMeta; -import org.springaicommunity.mcp.annotation.McpProgressToken; import org.springaicommunity.mcp.annotation.McpTool; import org.springaicommunity.mcp.method.tool.utils.JsonParser; - -import com.fasterxml.jackson.core.type.TypeReference; - -import io.modelcontextprotocol.spec.McpSchema.CallToolRequest; -import io.modelcontextprotocol.spec.McpSchema.CallToolResult; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -46,109 +37,16 @@ * McpTransportContext) * @author Christian Tzolov */ -public abstract class AbstractAsyncMcpToolMethodCallback { +public abstract class AbstractAsyncMcpToolMethodCallback extends AbstractMcpToolMethodCallback { protected final Class toolCallExceptionClass; - private static final TypeReference> MAP_TYPE_REFERENCE = new TypeReference>() { - // No implementation needed - }; - - protected final Method toolMethod; - - protected final Object toolObject; - - protected final ReturnMode returnMode; - protected AbstractAsyncMcpToolMethodCallback(ReturnMode returnMode, Method toolMethod, Object toolObject, Class toolCallExceptionClass) { - this.toolMethod = toolMethod; - this.toolObject = toolObject; - this.returnMode = returnMode; + super(returnMode, toolMethod, toolObject); this.toolCallExceptionClass = toolCallExceptionClass; } - /** - * Invokes the tool method with the provided arguments. - * @param methodArguments The arguments to pass to the method - * @return The result of the method invocation - * @throws IllegalStateException if the method cannot be accessed - * @throws RuntimeException if there's an error invoking the method - */ - protected Object callMethod(Object[] methodArguments) { - this.toolMethod.setAccessible(true); - - Object result; - try { - result = this.toolMethod.invoke(this.toolObject, methodArguments); - } - catch (IllegalAccessException ex) { - throw new IllegalStateException("Could not access method: " + ex.getMessage(), ex); - } - catch (InvocationTargetException ex) { - throw new RuntimeException("Error invoking method: " + this.toolMethod.getName(), ex); - } - return result; - } - - /** - * Builds the method arguments from the context, tool input arguments, and optionally - * the full request. - * @param exchangeOrContext The exchange or context object (e.g., - * McpAsyncServerExchange or McpTransportContext) - * @param toolInputArguments The input arguments from the tool request - * @param request The full CallToolRequest (optional, can be null) - * @return An array of method arguments - */ - protected Object[] buildMethodArguments(T exchangeOrContext, Map toolInputArguments, - CallToolRequest request) { - return Stream.of(this.toolMethod.getParameters()).map(parameter -> { - // Check if parameter is annotated with @McpProgressToken - if (parameter.isAnnotationPresent(McpProgressToken.class)) { - // Return the progress token from the request - return request != null ? request.progressToken() : null; - } - - // Check if parameter is McpMeta type - if (McpMeta.class.isAssignableFrom(parameter.getType())) { - // Return the meta from the request wrapped in McpMeta - return request != null ? new McpMeta(request.meta()) : new McpMeta(null); - } - - // Check if parameter is CallToolRequest type - if (CallToolRequest.class.isAssignableFrom(parameter.getType())) { - return request; - } - - if (isExchangeOrContextType(parameter.getType())) { - return exchangeOrContext; - } - - Object rawArgument = toolInputArguments.get(parameter.getName()); - return buildTypedArgument(rawArgument, parameter.getParameterizedType()); - }).toArray(); - } - - /** - * Builds a typed argument from a raw value and type information. - * @param value The raw value - * @param type The target type - * @return The typed argument - */ - protected Object buildTypedArgument(Object value, Type type) { - if (value == null) { - return null; - } - - if (type instanceof Class) { - return JsonParser.toTypedObject(value, (Class) type); - } - - // For generic types, use the fromJson method that accepts Type - String json = JsonParser.toJson(value); - return JsonParser.fromJson(json, type); - } - /** * Convert reactive types to Mono * @param result The result from the method invocation @@ -233,41 +131,13 @@ protected Mono convertToCallToolResult(Object result) { } /** - * Map individual values to CallToolResult + * Map individual values to CallToolResult This method delegates to the parent class's + * convertValueToCallToolResult method to avoid code duplication. * @param value The value to map * @return A CallToolResult representing the mapped value */ protected CallToolResult mapValueToCallToolResult(Object value) { - // Return the result if it's already a CallToolResult - if (value instanceof CallToolResult) { - return (CallToolResult) value; - } - - Type returnType = this.toolMethod.getGenericReturnType(); - - if (returnMode == ReturnMode.VOID || returnType == Void.TYPE || returnType == void.class) { - return CallToolResult.builder().addTextContent(JsonParser.toJson("Done")).build(); - } - - if (this.returnMode == ReturnMode.STRUCTURED) { - String jsonOutput = JsonParser.toJson(value); - Object structuredOutput = JsonParser.fromJson(jsonOutput, MAP_TYPE_REFERENCE); - return CallToolResult.builder().structuredContent(structuredOutput).build(); - } - - // Default to text output - if (value == null) { - return CallToolResult.builder().addTextContent("null").build(); - } - - // For string results in TEXT mode, return the string directly without JSON - // serialization - if (value instanceof String) { - return CallToolResult.builder().addTextContent((String) value).build(); - } - - // For other types, serialize to JSON - return CallToolResult.builder().addTextContent(JsonParser.toJson(value)).build(); + return convertValueToCallToolResult(value); } /** @@ -275,10 +145,11 @@ protected CallToolResult mapValueToCallToolResult(Object value) { * @param e The exception that occurred * @return A Mono representing the error */ - protected Mono createErrorResult(Exception e) { + protected Mono createAsyncErrorResult(Exception e) { + Throwable rootCause = findCauseUsingPlainJava(e); return Mono.just(CallToolResult.builder() .isError(true) - .addTextContent("Error invoking method: %s".formatted(e.getMessage())) + .addTextContent(rootCause.getMessage()) .build()); } diff --git a/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AbstractMcpToolMethodCallback.java b/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AbstractMcpToolMethodCallback.java new file mode 100644 index 0000000..f5fa170 --- /dev/null +++ b/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AbstractMcpToolMethodCallback.java @@ -0,0 +1,206 @@ +/* + * Copyright 2025-2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springaicommunity.mcp.method.tool; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Type; +import java.util.Map; +import java.util.Objects; +import java.util.stream.Stream; + +import io.modelcontextprotocol.spec.McpSchema.CallToolRequest; +import io.modelcontextprotocol.spec.McpSchema.CallToolResult; +import org.springaicommunity.mcp.annotation.McpMeta; +import org.springaicommunity.mcp.annotation.McpProgressToken; +import org.springaicommunity.mcp.annotation.McpTool; +import org.springaicommunity.mcp.method.tool.utils.JsonParser; + +/** + * Abstract base class for creating Function callbacks around tool methods. + * + * This class provides common functionality for converting methods annotated with + * {@link McpTool} into callback functions that can be used to handle tool requests. It + * contains all the shared logic between synchronous and asynchronous implementations. + * + * @param The type of the context parameter (e.g., McpTransportContext, + * McpSyncServerExchange, or McpAsyncServerExchange) + * @author Christian Tzolov + */ +public abstract class AbstractMcpToolMethodCallback { + + protected final Method toolMethod; + + protected final Object toolObject; + + protected final ReturnMode returnMode; + + protected AbstractMcpToolMethodCallback(ReturnMode returnMode, Method toolMethod, Object toolObject) { + this.toolMethod = toolMethod; + this.toolObject = toolObject; + this.returnMode = returnMode; + } + + /** + * Invokes the tool method with the provided arguments. + * @param methodArguments The arguments to pass to the method + * @return The result of the method invocation + * @throws IllegalStateException if the method cannot be accessed + * @throws RuntimeException if there's an error invoking the method + */ + protected Object callMethod(Object[] methodArguments) { + this.toolMethod.setAccessible(true); + + Object result; + try { + result = this.toolMethod.invoke(this.toolObject, methodArguments); + } + catch (IllegalAccessException ex) { + throw new RuntimeException("Failed to access tool method", ex); + } + catch (InvocationTargetException ex) { + throw new RuntimeException("Error invoking method: " + this.toolMethod.getName(), ex.getCause()); + } + return result; + } + + /** + * Builds the method arguments from the context, tool input arguments, and optionally + * the full request. + * @param exchangeOrContext The exchange or context object (e.g., + * McpSyncServerExchange, McpAsyncServerExchange, or McpTransportContext) + * @param toolInputArguments The input arguments from the tool request + * @param request The full CallToolRequest (optional, can be null) + * @return An array of method arguments + */ + protected Object[] buildMethodArguments(T exchangeOrContext, Map toolInputArguments, + CallToolRequest request) { + return Stream.of(this.toolMethod.getParameters()).map(parameter -> { + // Check if parameter is annotated with @McpProgressToken + if (parameter.isAnnotationPresent(McpProgressToken.class)) { + // Return the progress token from the request + return request != null ? request.progressToken() : null; + } + + // Check if parameter is McpMeta type + if (McpMeta.class.isAssignableFrom(parameter.getType())) { + // Return the meta from the request wrapped in McpMeta + return request != null ? new McpMeta(request.meta()) : new McpMeta(null); + } + + // Check if parameter is CallToolRequest type + if (CallToolRequest.class.isAssignableFrom(parameter.getType())) { + return request; + } + + if (isExchangeOrContextType(parameter.getType())) { + return exchangeOrContext; + } + + Object rawArgument = toolInputArguments.get(parameter.getName()); + return buildTypedArgument(rawArgument, parameter.getParameterizedType()); + }).toArray(); + } + + /** + * Builds a typed argument from a raw value and type information. + * @param value The raw value + * @param type The target type + * @return The typed argument + */ + protected Object buildTypedArgument(Object value, Type type) { + if (value == null) { + return null; + } + + if (type instanceof Class) { + return JsonParser.toTypedObject(value, (Class) type); + } + + // For generic types, use the fromJson method that accepts Type + String json = JsonParser.toJson(value); + return JsonParser.fromJson(json, type); + } + + /** + * Converts a method result value to a CallToolResult based on the return mode and + * type. This method contains the common logic for processing results that is shared + * between synchronous and asynchronous implementations. + * @param result The result value to convert + * @return A CallToolResult representing the processed result + */ + protected CallToolResult convertValueToCallToolResult(Object result) { + // Return the result if it's already a CallToolResult + if (result instanceof CallToolResult) { + return (CallToolResult) result; + } + + Type returnType = this.toolMethod.getGenericReturnType(); + + if (returnMode == ReturnMode.VOID || returnType == Void.TYPE || returnType == void.class) { + return CallToolResult.builder().addTextContent(JsonParser.toJson("Done")).build(); + } + + if (this.returnMode == ReturnMode.STRUCTURED) { + String jsonOutput = JsonParser.toJson(result); + Object structuredOutput = JsonParser.fromJson(jsonOutput, Object.class); + return CallToolResult.builder().structuredContent(structuredOutput).build(); + } + + // Default to text output + if (result == null) { + return CallToolResult.builder().addTextContent("null").build(); + } + + // For string results in TEXT mode, return the string directly without JSON + // serialization + if (result instanceof String) { + return CallToolResult.builder().addTextContent((String) result).build(); + } + + // For other types, serialize to JSON + return CallToolResult.builder().addTextContent(JsonParser.toJson(result)).build(); + } + + /** + * Creates the base error message for exceptions that occur during method invocation. + * @param e The exception that occurred + * @return The error message string + */ + protected String createErrorMessage(Throwable e) { + return "Error invoking method: %s".formatted(e.getMessage()); + } + + /** + * Determines if the given parameter type is an exchange or context type that should + * be injected. Subclasses must implement this method to specify which types are + * considered exchange or context types. + * @param paramType The parameter type to check + * @return true if the parameter type is an exchange or context type, false otherwise + */ + protected abstract boolean isExchangeOrContextType(Class paramType); + + protected Throwable findCauseUsingPlainJava(Throwable throwable) { + Objects.requireNonNull(throwable); + Throwable rootCause = throwable; + while (rootCause.getCause() != null && rootCause.getCause() != rootCause) { + rootCause = rootCause.getCause(); + } + return rootCause; + } + +} diff --git a/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AbstractSyncMcpToolMethodCallback.java b/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AbstractSyncMcpToolMethodCallback.java index b775033..d05d016 100644 --- a/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AbstractSyncMcpToolMethodCallback.java +++ b/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AbstractSyncMcpToolMethodCallback.java @@ -16,201 +16,61 @@ package org.springaicommunity.mcp.method.tool; -import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.lang.reflect.Type; -import java.util.Map; -import java.util.stream.Stream; -import com.fasterxml.jackson.core.type.TypeReference; import io.modelcontextprotocol.spec.McpSchema.CallToolRequest; import io.modelcontextprotocol.spec.McpSchema.CallToolResult; -import org.springaicommunity.mcp.annotation.McpMeta; -import org.springaicommunity.mcp.annotation.McpProgressToken; -import org.springaicommunity.mcp.annotation.McpTool; -import org.springaicommunity.mcp.method.tool.utils.JsonParser; /** - * Abstract base class for creating Function callbacks around tool methods. + * Abstract base class for creating Function callbacks around synchronous tool methods. * - * This class provides common functionality for converting methods annotated with - * {@link McpTool} into callback functions that can be used to handle tool requests. + * This class extends {@link AbstractAsyncMcpToolMethodCallback} and provides synchronous + * wrapper methods for handling tool requests. It converts the asynchronous reactive + * methods from the parent class into synchronous equivalents suitable for blocking + * operations. * * @param The type of the context parameter (e.g., McpTransportContext or * McpSyncServerExchange) * @author Christian Tzolov */ -public abstract class AbstractSyncMcpToolMethodCallback { - - protected final Class toolCallExceptionClass; - - private static final TypeReference> MAP_TYPE_REFERENCE = new TypeReference>() { - // No implementation needed - }; - - protected final Method toolMethod; - - protected final Object toolObject; - - protected final ReturnMode returnMode; +public abstract class AbstractSyncMcpToolMethodCallback extends AbstractAsyncMcpToolMethodCallback { protected AbstractSyncMcpToolMethodCallback(ReturnMode returnMode, Method toolMethod, Object toolObject, Class toolCallExceptionClass) { - this.toolMethod = toolMethod; - this.toolObject = toolObject; - this.returnMode = returnMode; - this.toolCallExceptionClass = toolCallExceptionClass; - } - - /** - * Invokes the tool method with the provided arguments. - * @param methodArguments The arguments to pass to the method - * @return The result of the method invocation - * @throws IllegalStateException if the method cannot be accessed - * @throws RuntimeException if there's an error invoking the method - */ - protected Object callMethod(Object[] methodArguments) { - this.toolMethod.setAccessible(true); - - Object result; - try { - result = this.toolMethod.invoke(this.toolObject, methodArguments); - } - catch (IllegalAccessException ex) { - throw new IllegalStateException("Could not access method: " + ex.getMessage(), ex); - } - catch (InvocationTargetException ex) { - throw new RuntimeException("Error invoking method: " + this.toolMethod.getName(), ex); - } - return result; - } - - /** - * Builds the method arguments from the context, tool input arguments, and optionally - * the full request. - * @param exchangeOrContext The exchange or context object (e.g., - * McpSyncServerExchange or McpTransportContext) - * @param toolInputArguments The input arguments from the tool request - * @param request The full CallToolRequest (optional, can be null) - * @return An array of method arguments - */ - protected Object[] buildMethodArguments(T exchangeOrContext, Map toolInputArguments, - CallToolRequest request) { - return Stream.of(this.toolMethod.getParameters()).map(parameter -> { - // Check if parameter is annotated with @McpProgressToken - if (parameter.isAnnotationPresent(McpProgressToken.class)) { - // Return the progress token from the request - return request != null ? request.progressToken() : null; - } - - // Check if parameter is McpMeta type - if (McpMeta.class.isAssignableFrom(parameter.getType())) { - // Return the meta from the request wrapped in McpMeta - return request != null ? new McpMeta(request.meta()) : new McpMeta(null); - } - - // Check if parameter is CallToolRequest type - if (CallToolRequest.class.isAssignableFrom(parameter.getType())) { - return request; - } - - if (isExchangeOrContextType(parameter.getType())) { - return exchangeOrContext; - } - - Object rawArgument = toolInputArguments.get(parameter.getName()); - return buildTypedArgument(rawArgument, parameter.getParameterizedType()); - }).toArray(); - } - - /** - * Builds a typed argument from a raw value and type information. - * @param value The raw value - * @param type The target type - * @return The typed argument - */ - protected Object buildTypedArgument(Object value, Type type) { - if (value == null) { - return null; - } - - if (type instanceof Class) { - return JsonParser.toTypedObject(value, (Class) type); - } - - // For generic types, use the fromJson method that accepts Type - String json = JsonParser.toJson(value); - return JsonParser.fromJson(json, type); + super(returnMode, toolMethod, toolObject, toolCallExceptionClass); } /** * Processes the result of the method invocation and converts it to a CallToolResult. + * This is a synchronous wrapper around the parent class's reactive result processing. * @param result The result from the method invocation * @return A CallToolResult representing the processed result */ protected CallToolResult processResult(Object result) { - // Return the result if it's already a CallToolResult - if (result instanceof CallToolResult) { - return (CallToolResult) result; - } - - Type returnType = this.toolMethod.getGenericReturnType(); - - if (returnMode == ReturnMode.VOID || returnType == Void.TYPE || returnType == void.class) { - return CallToolResult.builder().addTextContent(JsonParser.toJson("Done")).build(); - } - - if (this.returnMode == ReturnMode.STRUCTURED) { - String jsonOutput = JsonParser.toJson(result); - Object structuredOutput = JsonParser.fromJson(jsonOutput, Object.class); - return CallToolResult.builder().structuredContent(structuredOutput).build(); - } - - // Default to text output - if (result == null) { - return CallToolResult.builder().addTextContent("null").build(); - } - - // For string results in TEXT mode, return the string directly without JSON - // serialization - if (result instanceof String) { - return CallToolResult.builder().addTextContent((String) result).build(); - } - - // For other types, serialize to JSON - return CallToolResult.builder().addTextContent(JsonParser.toJson(result)).build(); + return mapValueToCallToolResult(result); } /** - * Creates an error result for exceptions that occur during method invocation. + * Creates an error result for exceptions that occur during method invocation. This is + * a synchronous wrapper around the parent class's reactive error handling. * @param e The exception that occurred * @return A CallToolResult representing the error */ - protected CallToolResult createErrorResult(Exception e) { - return CallToolResult.builder() - .isError(true) - .addTextContent("Error invoking method: %s".formatted(e.getMessage())) - .build(); + protected CallToolResult createSyncErrorResult(Exception e) { + Throwable rootCause = findCauseUsingPlainJava(e); + return CallToolResult.builder().isError(true).addTextContent(rootCause.getMessage()).build(); } /** - * Validates that the request is not null. + * Validates that the request is not null. This is a synchronous wrapper around the + * parent class's reactive validation. * @param request The request to validate * @throws IllegalArgumentException if the request is null */ - protected void validateRequest(CallToolRequest request) { + protected void validateSyncRequest(CallToolRequest request) { if (request == null) { throw new IllegalArgumentException("Request must not be null"); } } - /** - * Determines if the given parameter type is an exchange or context type that should - * be injected. Subclasses must implement this method to specify which types are - * considered exchange or context types. - * @param paramType The parameter type to check - * @return true if the parameter type is an exchange or context type, false otherwise - */ - protected abstract boolean isExchangeOrContextType(Class paramType); - } diff --git a/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AsyncMcpToolMethodCallback.java b/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AsyncMcpToolMethodCallback.java index 8d9bc42..ab51cab 100644 --- a/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AsyncMcpToolMethodCallback.java +++ b/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AsyncMcpToolMethodCallback.java @@ -78,7 +78,7 @@ public Mono apply(McpAsyncServerExchange exchange, CallToolReque } catch (Exception e) { if (this.toolCallExceptionClass.isInstance(e)) { - return this.createErrorResult(e); + return this.createAsyncErrorResult(e); } return Mono.error(e); } diff --git a/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AsyncStatelessMcpToolMethodCallback.java b/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AsyncStatelessMcpToolMethodCallback.java index f97f349..300f9cf 100644 --- a/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AsyncStatelessMcpToolMethodCallback.java +++ b/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AsyncStatelessMcpToolMethodCallback.java @@ -77,7 +77,7 @@ public Mono apply(McpTransportContext mcpTransportContext, CallT } catch (Exception e) { if (this.toolCallExceptionClass.isInstance(e)) { - return this.createErrorResult(e); + return this.createAsyncErrorResult(e); } return Mono.error(e); } diff --git a/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/SyncMcpToolMethodCallback.java b/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/SyncMcpToolMethodCallback.java index 344dee8..eaba1b2 100644 --- a/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/SyncMcpToolMethodCallback.java +++ b/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/SyncMcpToolMethodCallback.java @@ -60,7 +60,7 @@ protected boolean isExchangeOrContextType(Class paramType) { */ @Override public CallToolResult apply(McpSyncServerExchange exchange, CallToolRequest request) { - validateRequest(request); + validateSyncRequest(request); try { // Build arguments for the method call, passing the full request for @@ -75,7 +75,7 @@ public CallToolResult apply(McpSyncServerExchange exchange, CallToolRequest requ } catch (Exception e) { if (this.toolCallExceptionClass.isInstance(e)) { - return this.createErrorResult(e); + return this.createSyncErrorResult(e); } throw e; } diff --git a/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/SyncStatelessMcpToolMethodCallback.java b/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/SyncStatelessMcpToolMethodCallback.java index 4daee3e..a17cc9f 100644 --- a/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/SyncStatelessMcpToolMethodCallback.java +++ b/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/SyncStatelessMcpToolMethodCallback.java @@ -52,7 +52,7 @@ protected boolean isExchangeOrContextType(Class paramType) { @Override public CallToolResult apply(McpTransportContext mcpTransportContext, CallToolRequest callToolRequest) { - validateRequest(callToolRequest); + validateSyncRequest(callToolRequest); try { // Build arguments for the method call @@ -67,7 +67,7 @@ public CallToolResult apply(McpTransportContext mcpTransportContext, CallToolReq } catch (Exception e) { if (this.toolCallExceptionClass.isInstance(e)) { - return this.createErrorResult(e); + return this.createSyncErrorResult(e); } throw e; } diff --git a/mcp-annotations/src/test/java/org/springaicommunity/mcp/method/tool/SyncMcpToolMethodCallbackExceptionHandlingTests.java b/mcp-annotations/src/test/java/org/springaicommunity/mcp/method/tool/SyncMcpToolMethodCallbackExceptionHandlingTests.java index cde2f89..08a3037 100644 --- a/mcp-annotations/src/test/java/org/springaicommunity/mcp/method/tool/SyncMcpToolMethodCallbackExceptionHandlingTests.java +++ b/mcp-annotations/src/test/java/org/springaicommunity/mcp/method/tool/SyncMcpToolMethodCallbackExceptionHandlingTests.java @@ -16,7 +16,6 @@ package org.springaicommunity.mcp.method.tool; -import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.Map; @@ -112,7 +111,7 @@ public void testDefaultConstructor_CatchesAllExceptions() throws Exception { assertThat(result.isError()).isTrue(); assertThat(result.content()).hasSize(1); assertThat(result.content().get(0)).isInstanceOf(TextContent.class); - assertThat(((TextContent) result.content().get(0)).text()).contains("Error invoking method"); + assertThat(((TextContent) result.content().get(0)).text()).contains("Runtime error: test"); } @Test @@ -133,7 +132,7 @@ public void testExceptionClassConstructor_CatchesSpecifiedExceptions() throws Ex assertThat(result.isError()).isTrue(); assertThat(result.content()).hasSize(1); assertThat(result.content().get(0)).isInstanceOf(TextContent.class); - assertThat(((TextContent) result.content().get(0)).text()).contains("Error invoking method"); + assertThat(((TextContent) result.content().get(0)).text()).contains("Custom runtime error: test"); } @Test @@ -175,7 +174,7 @@ public void testCheckedExceptionHandling_WithExceptionClass() throws Exception { assertThat(result.isError()).isTrue(); assertThat(result.content()).hasSize(1); assertThat(result.content().get(0)).isInstanceOf(TextContent.class); - assertThat(((TextContent) result.content().get(0)).text()).contains("Error invoking method"); + assertThat(((TextContent) result.content().get(0)).text()).contains("Business error: test"); } @Test @@ -193,7 +192,7 @@ public void testCheckedExceptionHandling_WithSpecificClass() throws Exception { // The RuntimeException wrapper should NOT be caught assertThatThrownBy(() -> callback.apply(exchange, request)).isInstanceOf(RuntimeException.class) .hasMessageContaining("Error invoking method") - .hasCauseInstanceOf(InvocationTargetException.class); + .hasCauseInstanceOf(BusinessException.class); } @Test @@ -237,7 +236,7 @@ public void testNullPointerException_WithRuntimeExceptionClass() throws Exceptio assertThat(result.isError()).isTrue(); assertThat(result.content()).hasSize(1); assertThat(result.content().get(0)).isInstanceOf(TextContent.class); - assertThat(((TextContent) result.content().get(0)).text()).contains("Error invoking method"); + assertThat(((TextContent) result.content().get(0)).text()).contains("Null pointer: test"); } @Test @@ -259,7 +258,7 @@ public void testIllegalArgumentException_WithSpecificHandling() throws Exception assertThat(result.isError()).isTrue(); assertThat(result.content()).hasSize(1); assertThat(result.content().get(0)).isInstanceOf(TextContent.class); - assertThat(((TextContent) result.content().get(0)).text()).contains("Error invoking method"); + assertThat(((TextContent) result.content().get(0)).text()).contains("Illegal argument: test"); } @Test @@ -287,7 +286,7 @@ public void testMultipleCallsWithDifferentResults() throws Exception { CallToolRequest exceptionRequest = new CallToolRequest("runtime-exception-tool", Map.of("input", "error")); CallToolResult exceptionResult = exceptionCallback.apply(exchange, exceptionRequest); assertThat(exceptionResult.isError()).isTrue(); - assertThat(((TextContent) exceptionResult.content().get(0)).text()).contains("Error invoking method"); + assertThat(((TextContent) exceptionResult.content().get(0)).text()).contains("Runtime error: error"); } @Test diff --git a/mcp-annotations/src/test/java/org/springaicommunity/mcp/method/tool/SyncMcpToolMethodCallbackTests.java b/mcp-annotations/src/test/java/org/springaicommunity/mcp/method/tool/SyncMcpToolMethodCallbackTests.java index 6b14ed9..1f84686 100644 --- a/mcp-annotations/src/test/java/org/springaicommunity/mcp/method/tool/SyncMcpToolMethodCallbackTests.java +++ b/mcp-annotations/src/test/java/org/springaicommunity/mcp/method/tool/SyncMcpToolMethodCallbackTests.java @@ -364,10 +364,7 @@ public void testToolThatThrowsException() throws Exception { assertThat(result.isError()).isTrue(); assertThat(result.content()).hasSize(1); assertThat(result.content().get(0)).isInstanceOf(TextContent.class); - assertThat(((TextContent) result.content().get(0)).text()).contains("Error invoking method"); - // The actual error message format may be different, so let's just check for the - // method name - assertThat(((TextContent) result.content().get(0)).text()).contains("exceptionTool"); + assertThat(((TextContent) result.content().get(0)).text()).contains("Tool execution failed: test"); } @Test @@ -468,7 +465,8 @@ public void testToolWithInvalidJsonConversion() throws Exception { assertThat(result.isError()).isTrue(); assertThat(result.content()).hasSize(1); assertThat(result.content().get(0)).isInstanceOf(TextContent.class); - assertThat(((TextContent) result.content().get(0)).text()).contains("Error invoking method"); + assertThat(((TextContent) result.content().get(0)).text()).contains( + "Cannot construct instance of `org.springaicommunity.mcp.method.tool.SyncMcpToolMethodCallbackTests$TestObject` "); } @Test diff --git a/mcp-annotations/src/test/java/org/springaicommunity/mcp/method/tool/SyncStatelessMcpToolMethodCallbackTests.java b/mcp-annotations/src/test/java/org/springaicommunity/mcp/method/tool/SyncStatelessMcpToolMethodCallbackTests.java index b1ea1f8..9f7120a 100644 --- a/mcp-annotations/src/test/java/org/springaicommunity/mcp/method/tool/SyncStatelessMcpToolMethodCallbackTests.java +++ b/mcp-annotations/src/test/java/org/springaicommunity/mcp/method/tool/SyncStatelessMcpToolMethodCallbackTests.java @@ -399,8 +399,7 @@ public void testToolThatThrowsException() throws Exception { assertThat(result.isError()).isTrue(); assertThat(result.content()).hasSize(1); assertThat(result.content().get(0)).isInstanceOf(TextContent.class); - assertThat(((TextContent) result.content().get(0)).text()).contains("Error invoking method"); - assertThat(((TextContent) result.content().get(0)).text()).contains("exceptionTool"); + assertThat(((TextContent) result.content().get(0)).text()).contains("Tool execution failed: test"); } @Test @@ -512,7 +511,8 @@ public void testToolWithInvalidJsonConversion() throws Exception { assertThat(result.isError()).isTrue(); assertThat(result.content()).hasSize(1); assertThat(result.content().get(0)).isInstanceOf(TextContent.class); - assertThat(((TextContent) result.content().get(0)).text()).contains("Error invoking method"); + assertThat(((TextContent) result.content().get(0)).text()).contains( + "Cannot construct instance of `org.springaicommunity.mcp.method.tool.SyncStatelessMcpToolMethodCallbackTests$TestObject`"); } @Test From a91448144d83a058cc3ca8d08803c021155f2b01 Mon Sep 17 00:00:00 2001 From: Christian Tzolov Date: Thu, 18 Sep 2025 15:28:48 +0200 Subject: [PATCH 2/3] fix formatting Signed-off-by: Christian Tzolov --- .../mcp/method/tool/AbstractAsyncMcpToolMethodCallback.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AbstractAsyncMcpToolMethodCallback.java b/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AbstractAsyncMcpToolMethodCallback.java index c73554e..b1e19be 100644 --- a/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AbstractAsyncMcpToolMethodCallback.java +++ b/mcp-annotations/src/main/java/org/springaicommunity/mcp/method/tool/AbstractAsyncMcpToolMethodCallback.java @@ -147,10 +147,7 @@ protected CallToolResult mapValueToCallToolResult(Object value) { */ protected Mono createAsyncErrorResult(Exception e) { Throwable rootCause = findCauseUsingPlainJava(e); - return Mono.just(CallToolResult.builder() - .isError(true) - .addTextContent(rootCause.getMessage()) - .build()); + return Mono.just(CallToolResult.builder().isError(true).addTextContent(rootCause.getMessage()).build()); } /** From 303417a58f9c368f038227c817f82b462660466d Mon Sep 17 00:00:00 2001 From: Christian Tzolov Date: Thu, 18 Sep 2025 15:33:34 +0200 Subject: [PATCH 3/3] fix failing tests Signed-off-by: Christian Tzolov --- .../mcp/method/tool/AsyncMcpToolMethodCallbackTests.java | 9 +++++---- .../tool/AsyncStatelessMcpToolMethodCallbackTests.java | 9 +++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/mcp-annotations/src/test/java/org/springaicommunity/mcp/method/tool/AsyncMcpToolMethodCallbackTests.java b/mcp-annotations/src/test/java/org/springaicommunity/mcp/method/tool/AsyncMcpToolMethodCallbackTests.java index d65adbb..0f5d9cf 100644 --- a/mcp-annotations/src/test/java/org/springaicommunity/mcp/method/tool/AsyncMcpToolMethodCallbackTests.java +++ b/mcp-annotations/src/test/java/org/springaicommunity/mcp/method/tool/AsyncMcpToolMethodCallbackTests.java @@ -486,8 +486,7 @@ public void testMonoToolThatReturnsNull() throws Exception { assertThat(result.isError()).isTrue(); assertThat(result.content()).hasSize(1); assertThat(result.content().get(0)).isInstanceOf(TextContent.class); - assertThat(((TextContent) result.content().get(0)).text()) - .isEqualTo("Error invoking method: Error invoking method: nullReturnMonoTool"); + assertThat(((TextContent) result.content().get(0)).text()).isEqualTo("value"); }).verifyComplete(); } @@ -620,7 +619,8 @@ public void testNonReactiveToolShouldFail() throws Exception { assertThat(result.isError()).isTrue(); assertThat(result.content()).hasSize(1); assertThat(result.content().get(0)).isInstanceOf(TextContent.class); - assertThat(((TextContent) result.content().get(0)).text()).contains("Error invoking method"); + assertThat(((TextContent) result.content().get(0)).text()) + .contains("Expected reactive return type but got: java.lang.String"); }).verifyComplete(); } @@ -639,7 +639,8 @@ public void testMonoToolWithInvalidJsonConversion() throws Exception { assertThat(result.isError()).isTrue(); assertThat(result.content()).hasSize(1); assertThat(result.content().get(0)).isInstanceOf(TextContent.class); - assertThat(((TextContent) result.content().get(0)).text()).contains("Error invoking method"); + assertThat(((TextContent) result.content().get(0)).text()).contains( + "Cannot construct instance of `org.springaicommunity.mcp.method.tool.AsyncMcpToolMethodCallbackTests$TestObject`"); }).verifyComplete(); } diff --git a/mcp-annotations/src/test/java/org/springaicommunity/mcp/method/tool/AsyncStatelessMcpToolMethodCallbackTests.java b/mcp-annotations/src/test/java/org/springaicommunity/mcp/method/tool/AsyncStatelessMcpToolMethodCallbackTests.java index 5dccd71..42f0e60 100644 --- a/mcp-annotations/src/test/java/org/springaicommunity/mcp/method/tool/AsyncStatelessMcpToolMethodCallbackTests.java +++ b/mcp-annotations/src/test/java/org/springaicommunity/mcp/method/tool/AsyncStatelessMcpToolMethodCallbackTests.java @@ -517,8 +517,7 @@ public void testMonoToolThatReturnsNull() throws Exception { assertThat(result.isError()).isTrue(); assertThat(result.content()).hasSize(1); assertThat(result.content().get(0)).isInstanceOf(TextContent.class); - assertThat(((TextContent) result.content().get(0)).text()) - .isEqualTo("Error invoking method: Error invoking method: nullReturnMonoTool"); + assertThat(((TextContent) result.content().get(0)).text()).isEqualTo("value"); }).verifyComplete(); } @@ -659,7 +658,8 @@ public void testNonReactiveToolShouldFail() throws Exception { assertThat(result.isError()).isTrue(); assertThat(result.content()).hasSize(1); assertThat(result.content().get(0)).isInstanceOf(TextContent.class); - assertThat(((TextContent) result.content().get(0)).text()).contains("Error invoking method"); + assertThat(((TextContent) result.content().get(0)).text()) + .contains("Expected reactive return type but got: java.lang.String"); }).verifyComplete(); } @@ -679,7 +679,8 @@ public void testMonoToolWithInvalidJsonConversion() throws Exception { assertThat(result.isError()).isTrue(); assertThat(result.content()).hasSize(1); assertThat(result.content().get(0)).isInstanceOf(TextContent.class); - assertThat(((TextContent) result.content().get(0)).text()).contains("Error invoking method"); + assertThat(((TextContent) result.content().get(0)).text()).contains( + "Cannot construct instance of `org.springaicommunity.mcp.method.tool.AsyncStatelessMcpToolMethodCallbackTests$TestObject`"); }).verifyComplete(); }