diff --git a/spring-ai-model/src/main/java/org/springframework/ai/tool/execution/ToolParameterConversionException.java b/spring-ai-model/src/main/java/org/springframework/ai/tool/execution/ToolParameterConversionException.java new file mode 100644 index 00000000000..74b4125b5cd --- /dev/null +++ b/spring-ai-model/src/main/java/org/springframework/ai/tool/execution/ToolParameterConversionException.java @@ -0,0 +1,172 @@ +/* + * Copyright 2023-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.springframework.ai.tool.execution; + +import java.lang.reflect.Type; + +import org.springframework.ai.tool.definition.ToolDefinition; +import org.springframework.lang.Nullable; + +/** + * An exception thrown when a tool parameter conversion fails, typically when the model + * provides a value that cannot be converted to the expected parameter type. + * + *

+ * This exception provides detailed context about the conversion failure including: + *

+ * + * @author Christian Tzolov + * @since 1.1.0 + */ +public class ToolParameterConversionException extends ToolExecutionException { + + @Nullable + private final String parameterName; + + private final Type expectedType; + + @Nullable + private final Object actualValue; + + /** + * Creates a new ToolParameterConversionException with detailed parameter context. + * @param toolDefinition the tool definition where the conversion failed + * @param parameterName the name of the parameter that failed conversion (may be null + * if not available) + * @param expectedType the expected parameter type + * @param actualValue the actual value provided by the model that failed conversion + * @param cause the underlying exception that caused the conversion failure + */ + public ToolParameterConversionException(ToolDefinition toolDefinition, @Nullable String parameterName, + Type expectedType, @Nullable Object actualValue, Throwable cause) { + super(toolDefinition, new RuntimeException( + buildMessage(toolDefinition, parameterName, expectedType, actualValue, cause), cause)); + this.parameterName = parameterName; + this.expectedType = expectedType; + this.actualValue = actualValue; + } + + /** + * Creates a new ToolParameterConversionException without parameter name context. + * @param toolDefinition the tool definition where the conversion failed + * @param expectedType the expected parameter type + * @param actualValue the actual value provided by the model that failed conversion + * @param cause the underlying exception that caused the conversion failure + */ + public ToolParameterConversionException(ToolDefinition toolDefinition, Type expectedType, + @Nullable Object actualValue, Throwable cause) { + this(toolDefinition, null, expectedType, actualValue, cause); + } + + private static String buildMessage(ToolDefinition toolDefinition, @Nullable String parameterName, Type expectedType, + @Nullable Object actualValue, Throwable cause) { + + StringBuilder message = new StringBuilder("Tool parameter conversion failed"); + + if (parameterName != null) { + message.append(" for parameter '").append(parameterName).append("'"); + } + + message.append(" in tool '").append(toolDefinition.name()).append("': "); + + String typeName = expectedType instanceof Class ? ((Class) expectedType).getSimpleName() + : expectedType.getTypeName(); + message.append("Expected type: ").append(typeName); + + if (actualValue != null) { + if (actualValue instanceof String && ((String) actualValue).isEmpty()) { + message.append(", but received: \"\" (empty string)"); + } + else { + String valueStr = actualValue.toString(); + if (valueStr.length() > 50) { + valueStr = valueStr.substring(0, 47) + "..."; + } + message.append(", but received: \"").append(valueStr).append("\""); + message.append(" (").append(actualValue.getClass().getSimpleName()).append(")"); + } + } + else { + message.append(", but received: null"); + } + + // Add helpful suggestions + message.append(". "); + if (isNumericType(expectedType) && actualValue instanceof String && ((String) actualValue).isEmpty()) { + message.append( + "Suggestion: Ensure your prompt clearly specifies that numeric parameters should contain valid numbers, not empty strings. "); + message.append( + "Consider making the parameter optional or providing a default value in your tool description."); + } + else if (isNumericType(expectedType)) { + message.append("Suggestion: Verify that the model is providing numeric values for numeric parameters. "); + message.append("Review your tool description and prompt to ensure clarity about expected parameter types."); + } + else { + message.append( + "Suggestion: Review your tool description and prompt to ensure the model provides values compatible with the expected parameter type."); + } + + if (cause != null && cause.getMessage() != null) { + message.append(" Original error: ").append(cause.getMessage()); + } + + return message.toString(); + } + + private static boolean isNumericType(Type type) { + if (type instanceof Class) { + Class clazz = (Class) type; + return clazz == Byte.class || clazz == byte.class || clazz == Short.class || clazz == short.class + || clazz == Integer.class || clazz == int.class || clazz == Long.class || clazz == long.class + || clazz == Float.class || clazz == float.class || clazz == Double.class || clazz == double.class; + } + return false; + } + + /** + * Returns the name of the parameter that failed conversion. + * @return the parameter name, or null if not available + */ + @Nullable + public String getParameterName() { + return this.parameterName; + } + + /** + * Returns the expected parameter type. + * @return the expected type + */ + public Type getExpectedType() { + return this.expectedType; + } + + /** + * Returns the actual value provided by the model that failed conversion. + * @return the actual value, or null if the value was null + */ + @Nullable + public Object getActualValue() { + return this.actualValue; + } + +} \ No newline at end of file diff --git a/spring-ai-model/src/main/java/org/springframework/ai/tool/method/MethodToolCallback.java b/spring-ai-model/src/main/java/org/springframework/ai/tool/method/MethodToolCallback.java index 7c303f3a693..fd2819a61ee 100644 --- a/spring-ai-model/src/main/java/org/springframework/ai/tool/method/MethodToolCallback.java +++ b/spring-ai-model/src/main/java/org/springframework/ai/tool/method/MethodToolCallback.java @@ -33,6 +33,7 @@ import org.springframework.ai.tool.execution.DefaultToolCallResultConverter; import org.springframework.ai.tool.execution.ToolCallResultConverter; import org.springframework.ai.tool.execution.ToolExecutionException; +import org.springframework.ai.tool.execution.ToolParameterConversionException; import org.springframework.ai.tool.metadata.ToolMetadata; import org.springframework.ai.util.json.JsonParser; import org.springframework.lang.Nullable; @@ -136,23 +137,28 @@ private Object[] buildMethodArguments(Map toolInputArguments, @N return toolContext; } Object rawArgument = toolInputArguments.get(parameter.getName()); - return buildTypedArgument(rawArgument, parameter.getParameterizedType()); + return buildTypedArgument(rawArgument, parameter.getParameterizedType(), parameter.getName()); }).toArray(); } @Nullable - private Object buildTypedArgument(@Nullable Object value, Type type) { + private Object buildTypedArgument(@Nullable Object value, Type type, @Nullable String parameterName) { if (value == null) { return null; } - if (type instanceof Class) { - return JsonParser.toTypedObject(value, (Class) type); - } + try { + 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); + // For generic types, use the fromJson method that accepts Type + String json = JsonParser.toJson(value); + return JsonParser.fromJson(json, type); + } + catch (NumberFormatException | IllegalStateException ex) { + throw new ToolParameterConversionException(this.toolDefinition, parameterName, type, value, ex); + } } @Nullable diff --git a/spring-ai-model/src/main/java/org/springframework/ai/util/json/JsonParser.java b/spring-ai-model/src/main/java/org/springframework/ai/util/json/JsonParser.java index b5448803152..a9e6383fa9e 100644 --- a/spring-ai-model/src/main/java/org/springframework/ai/util/json/JsonParser.java +++ b/spring-ai-model/src/main/java/org/springframework/ai/util/json/JsonParser.java @@ -137,39 +137,155 @@ public static Object toTypedObject(Object value, Class type) { Assert.notNull(type, "type cannot be null"); var javaType = ClassUtils.resolvePrimitiveIfNecessary(type); + String valueString = value.toString(); if (javaType == String.class) { - return value.toString(); + return valueString; } else if (javaType == Byte.class) { - return Byte.parseByte(value.toString()); + return parseByteWithContext(valueString, javaType); } else if (javaType == Integer.class) { - BigDecimal bigDecimal = new BigDecimal(value.toString()); - return bigDecimal.intValueExact(); + return parseIntegerWithContext(valueString, javaType); } else if (javaType == Short.class) { - return Short.parseShort(value.toString()); + return parseShortWithContext(valueString, javaType); } else if (javaType == Long.class) { - BigDecimal bigDecimal = new BigDecimal(value.toString()); - return bigDecimal.longValueExact(); + return parseLongWithContext(valueString, javaType); } else if (javaType == Double.class) { - return Double.parseDouble(value.toString()); + return parseDoubleWithContext(valueString, javaType); } else if (javaType == Float.class) { - return Float.parseFloat(value.toString()); + return parseFloatWithContext(valueString, javaType); } else if (javaType == Boolean.class) { - return Boolean.parseBoolean(value.toString()); + return Boolean.parseBoolean(valueString); } else if (javaType.isEnum()) { - return Enum.valueOf((Class) javaType, value.toString()); + return parseEnumWithContext(valueString, (Class) javaType); } String json = JsonParser.toJson(value); return JsonParser.fromJson(json, javaType); } + private static Byte parseByteWithContext(String value, Class targetType) { + try { + return Byte.parseByte(value); + } + catch (NumberFormatException ex) { + throw new NumberFormatException(buildNumberFormatMessage(value, targetType, ex)); + } + } + + private static Integer parseIntegerWithContext(String value, Class targetType) { + try { + BigDecimal bigDecimal = new BigDecimal(value); + return bigDecimal.intValueExact(); + } + catch (NumberFormatException ex) { + throw new NumberFormatException(buildNumberFormatMessage(value, targetType, ex)); + } + } + + private static Short parseShortWithContext(String value, Class targetType) { + try { + return Short.parseShort(value); + } + catch (NumberFormatException ex) { + throw new NumberFormatException(buildNumberFormatMessage(value, targetType, ex)); + } + } + + private static Long parseLongWithContext(String value, Class targetType) { + try { + BigDecimal bigDecimal = new BigDecimal(value); + return bigDecimal.longValueExact(); + } + catch (NumberFormatException ex) { + throw new NumberFormatException(buildNumberFormatMessage(value, targetType, ex)); + } + } + + private static Double parseDoubleWithContext(String value, Class targetType) { + try { + return Double.parseDouble(value); + } + catch (NumberFormatException ex) { + throw new NumberFormatException(buildNumberFormatMessage(value, targetType, ex)); + } + } + + private static Float parseFloatWithContext(String value, Class targetType) { + try { + return Float.parseFloat(value); + } + catch (NumberFormatException ex) { + throw new NumberFormatException(buildNumberFormatMessage(value, targetType, ex)); + } + } + + @SuppressWarnings({ "rawtypes", "unchecked" }) + private static Enum parseEnumWithContext(String value, Class enumType) { + try { + return Enum.valueOf(enumType, value); + } + catch (IllegalArgumentException ex) { + throw new IllegalArgumentException(buildEnumFormatMessage(value, enumType, ex), ex); + } + } + + private static String buildNumberFormatMessage(String value, Class targetType, + NumberFormatException originalException) { + StringBuilder message = new StringBuilder("Cannot convert value to ").append(targetType.getSimpleName()) + .append(": "); + + if (value.isEmpty()) { + message.append("empty string provided"); + } + else { + message.append("'").append(value).append("'"); + } + + message.append(". Expected a valid ").append(targetType.getSimpleName().toLowerCase()).append(" value"); + + // Only append original exception message if it's not redundant + if (originalException.getMessage() != null && !isRedundantErrorMessage(originalException.getMessage(), value)) { + message.append(". ").append(originalException.getMessage()); + } + + return message.toString(); + } + + private static boolean isRedundantErrorMessage(String errorMessage, String originalValue) { + return errorMessage.equalsIgnoreCase("empty string") + || (originalValue.isEmpty() && errorMessage.equals("For input string: \"\"")); + } + + private static String buildEnumFormatMessage(String value, Class enumType, + IllegalArgumentException originalException) { + StringBuilder message = new StringBuilder("Cannot convert value to ").append(enumType.getSimpleName()) + .append(": "); + + if (value.isEmpty()) { + message.append("empty string provided"); + } + else { + message.append("'").append(value).append("'"); + } + + message.append(". Expected one of: "); + Enum[] constants = enumType.getEnumConstants(); + for (int i = 0; i < constants.length; i++) { + if (i > 0) { + message.append(", "); + } + message.append(constants[i].name()); + } + + return message.toString(); + } + } diff --git a/spring-ai-model/src/test/java/org/springframework/ai/tool/execution/ToolParameterConversionExceptionTest.java b/spring-ai-model/src/test/java/org/springframework/ai/tool/execution/ToolParameterConversionExceptionTest.java new file mode 100644 index 00000000000..65b365cf311 --- /dev/null +++ b/spring-ai-model/src/test/java/org/springframework/ai/tool/execution/ToolParameterConversionExceptionTest.java @@ -0,0 +1,111 @@ +/* + * Copyright 2023-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.springframework.ai.tool.execution; + +import org.junit.jupiter.api.Test; + +import org.springframework.ai.tool.definition.DefaultToolDefinition; +import org.springframework.ai.tool.definition.ToolDefinition; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link ToolParameterConversionException}. + * + * @author Christian Tzolov + */ +class ToolParameterConversionExceptionTest { + + private static final ToolDefinition TEST_TOOL = DefaultToolDefinition.builder() + .name("testTool") + .description("Test tool for conversion testing") + .inputSchema("{\"type\":\"object\",\"properties\":{}}") + .build(); + + @Test + void shouldCreateExceptionWithParameterName() { + var originalException = new NumberFormatException("For input string: \"\""); + var exception = new ToolParameterConversionException(TEST_TOOL, "userId", Long.class, "", originalException); + + assertThat(exception.getParameterName()).isEqualTo("userId"); + assertThat(exception.getExpectedType()).isEqualTo(Long.class); + assertThat(exception.getActualValue()).isEqualTo(""); + assertThat(exception.getToolDefinition()).isEqualTo(TEST_TOOL); + assertThat(exception.getCause()).isInstanceOf(RuntimeException.class); + assertThat(exception.getMessage()) + .contains("Tool parameter conversion failed for parameter 'userId' in tool 'testTool'") + .contains("Expected type: Long") + .contains("but received: \"\" (empty string)") + .contains( + "Suggestion: Ensure your prompt clearly specifies that numeric parameters should contain valid numbers"); + } + + @Test + void shouldCreateExceptionWithoutParameterName() { + var originalException = new NumberFormatException("Invalid number"); + var exception = new ToolParameterConversionException(TEST_TOOL, Integer.class, "invalid", originalException); + + assertThat(exception.getParameterName()).isNull(); + assertThat(exception.getExpectedType()).isEqualTo(Integer.class); + assertThat(exception.getActualValue()).isEqualTo("invalid"); + assertThat(exception.getMessage()).contains("Tool parameter conversion failed in tool 'testTool'") + .contains("Expected type: Integer") + .contains("but received: \"invalid\" (String)") + .contains("Suggestion: Verify that the model is providing numeric values"); + } + + @Test + void shouldCreateExceptionWithNullValue() { + var originalException = new NullPointerException("Null value"); + var exception = new ToolParameterConversionException(TEST_TOOL, "param", String.class, null, originalException); + + assertThat(exception.getActualValue()).isNull(); + assertThat(exception.getMessage()).contains("but received: null"); + } + + @Test + void shouldTruncateLongValues() { + String longValue = "This is a very long string that should be truncated because it exceeds the maximum length"; + var originalException = new IllegalArgumentException("Invalid value"); + var exception = new ToolParameterConversionException(TEST_TOOL, "param", String.class, longValue, + originalException); + + assertThat(exception.getMessage()).contains("This is a very long string that should be trunc...") + .doesNotContain("because it exceeds the maximum length"); + } + + @Test + void shouldProvideHelpfulSuggestionsForNumericTypes() { + var originalException = new NumberFormatException("Empty string"); + var exception = new ToolParameterConversionException(TEST_TOOL, "count", Long.class, "", originalException); + + assertThat(exception.getMessage()).contains( + "Suggestion: Ensure your prompt clearly specifies that numeric parameters should contain valid numbers") + .contains("Consider making the parameter optional or providing a default value"); + } + + @Test + void shouldProvideGenericSuggestionsForNonNumericTypes() { + var originalException = new IllegalArgumentException("Invalid format"); + var exception = new ToolParameterConversionException(TEST_TOOL, "data", String.class, new Object(), + originalException); + + assertThat(exception.getMessage()).contains( + "Suggestion: Review your tool description and prompt to ensure the model provides values compatible"); + } + +} \ No newline at end of file diff --git a/spring-ai-model/src/test/java/org/springframework/ai/util/json/JsonParserTests.java b/spring-ai-model/src/test/java/org/springframework/ai/util/json/JsonParserTests.java index 61e073b701b..dfd52319167 100644 --- a/spring-ai-model/src/test/java/org/springframework/ai/util/json/JsonParserTests.java +++ b/spring-ai-model/src/test/java/org/springframework/ai/util/json/JsonParserTests.java @@ -262,6 +262,69 @@ void doesNotDoubleSerializeValidJsonString() { assertThat(input).isEqualTo(result); } + @Test + void shouldThrowNumberFormatExceptionForEmptyStringToLong() { + // Reproduces GitHub issue #3884 + // When LLM returns empty string for Long parameter, conversion fails with + // helpful error message + assertThatThrownBy(() -> JsonParser.toTypedObject("", Long.class)).isInstanceOf(NumberFormatException.class) + .hasMessage("Cannot convert value to Long: empty string provided. Expected a valid long value"); + } + + @Test + void shouldThrowNumberFormatExceptionForEmptyStringToInteger() { + // Same issue applies to other numeric types - now with helpful error messages + assertThatThrownBy(() -> JsonParser.toTypedObject("", Integer.class)).isInstanceOf(NumberFormatException.class) + .hasMessage("Cannot convert value to Integer: empty string provided. Expected a valid integer value"); + } + + @Test + void shouldThrowNumberFormatExceptionForInvalidStringToLong() { + // Test invalid string conversion with improved error message + assertThatThrownBy(() -> JsonParser.toTypedObject("not-a-number", Long.class)) + .isInstanceOf(NumberFormatException.class) + .hasMessageContaining("Cannot convert value to Long: 'not-a-number'") + .hasMessageContaining("Expected a valid long value"); + } + + @Test + void shouldThrowNumberFormatExceptionForEmptyStringToByte() { + assertThatThrownBy(() -> JsonParser.toTypedObject("", Byte.class)).isInstanceOf(NumberFormatException.class) + .hasMessage("Cannot convert value to Byte: empty string provided. Expected a valid byte value"); + } + + @Test + void shouldThrowNumberFormatExceptionForEmptyStringToShort() { + assertThatThrownBy(() -> JsonParser.toTypedObject("", Short.class)).isInstanceOf(NumberFormatException.class) + .hasMessage("Cannot convert value to Short: empty string provided. Expected a valid short value"); + } + + @Test + void shouldThrowNumberFormatExceptionForEmptyStringToDouble() { + assertThatThrownBy(() -> JsonParser.toTypedObject("", Double.class)).isInstanceOf(NumberFormatException.class) + .hasMessage("Cannot convert value to Double: empty string provided. Expected a valid double value"); + } + + @Test + void shouldThrowNumberFormatExceptionForEmptyStringToFloat() { + assertThatThrownBy(() -> JsonParser.toTypedObject("", Float.class)).isInstanceOf(NumberFormatException.class) + .hasMessage("Cannot convert value to Float: empty string provided. Expected a valid float value"); + } + + @Test + void shouldThrowIllegalArgumentExceptionForInvalidEnum() { + assertThatThrownBy(() -> JsonParser.toTypedObject("INVALID", TestEnum.class)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot convert value to TestEnum: 'INVALID'. Expected one of: VALUE"); + } + + @Test + void shouldThrowIllegalArgumentExceptionForEmptyStringToEnum() { + assertThatThrownBy(() -> JsonParser.toTypedObject("", TestEnum.class)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot convert value to TestEnum: empty string provided. Expected one of: VALUE"); + } + record TestRecord(String name, Integer age) { }