Clarify tool validation error messages (#1023)
- DefaultJsonSchemaValidator now emits a neutral message; - Call sites in ToolInputValidator and the server output handlers prepend their own context prefix, - Integration test assertions strengthened accordingly. Closes #986 Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com> * address review comments Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com> * address review comments Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com> --------- Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
This commit is contained in:
parent
df75857fb1
commit
99faac65f7
8 changed files with 30 additions and 25 deletions
|
|
@ -26,7 +26,6 @@ import io.modelcontextprotocol.spec.McpSchema;
|
||||||
import io.modelcontextprotocol.spec.McpSchema.CallToolResult;
|
import io.modelcontextprotocol.spec.McpSchema.CallToolResult;
|
||||||
import io.modelcontextprotocol.spec.McpSchema.CompleteResult.CompleteCompletion;
|
import io.modelcontextprotocol.spec.McpSchema.CompleteResult.CompleteCompletion;
|
||||||
import io.modelcontextprotocol.spec.McpSchema.ErrorCodes;
|
import io.modelcontextprotocol.spec.McpSchema.ErrorCodes;
|
||||||
import io.modelcontextprotocol.spec.McpSchema.LoggingLevel;
|
|
||||||
import io.modelcontextprotocol.spec.McpSchema.PromptReference;
|
import io.modelcontextprotocol.spec.McpSchema.PromptReference;
|
||||||
import io.modelcontextprotocol.spec.McpSchema.ResourceReference;
|
import io.modelcontextprotocol.spec.McpSchema.ResourceReference;
|
||||||
import io.modelcontextprotocol.spec.McpSchema.SetLevelRequest;
|
import io.modelcontextprotocol.spec.McpSchema.SetLevelRequest;
|
||||||
|
|
@ -433,9 +432,11 @@ public class McpAsyncServer {
|
||||||
var validation = this.jsonSchemaValidator.validate(outputSchema, result.structuredContent());
|
var validation = this.jsonSchemaValidator.validate(outputSchema, result.structuredContent());
|
||||||
|
|
||||||
if (!validation.valid()) {
|
if (!validation.valid()) {
|
||||||
logger.warn("Tool call result validation failed: {}", validation.errorMessage());
|
String message = "Tool (" + request.name() + ") output validation failed: "
|
||||||
|
+ validation.errorMessage();
|
||||||
|
logger.warn(message);
|
||||||
return CallToolResult.builder()
|
return CallToolResult.builder()
|
||||||
.content(List.of(McpSchema.TextContent.builder(validation.errorMessage()).build()))
|
.content(List.of(McpSchema.TextContent.builder(message).build()))
|
||||||
.isError(true)
|
.isError(true)
|
||||||
.build();
|
.build();
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -4,9 +4,19 @@
|
||||||
|
|
||||||
package io.modelcontextprotocol.server;
|
package io.modelcontextprotocol.server;
|
||||||
|
|
||||||
import io.modelcontextprotocol.json.TypeRef;
|
import java.time.Duration;
|
||||||
import io.modelcontextprotocol.json.McpJsonMapper;
|
import java.util.ArrayList;
|
||||||
|
import java.util.HashMap;
|
||||||
|
import java.util.List;
|
||||||
|
import java.util.Map;
|
||||||
|
import java.util.Optional;
|
||||||
|
import java.util.concurrent.ConcurrentHashMap;
|
||||||
|
import java.util.concurrent.CopyOnWriteArrayList;
|
||||||
|
import java.util.function.BiFunction;
|
||||||
|
|
||||||
import io.modelcontextprotocol.common.McpTransportContext;
|
import io.modelcontextprotocol.common.McpTransportContext;
|
||||||
|
import io.modelcontextprotocol.json.McpJsonMapper;
|
||||||
|
import io.modelcontextprotocol.json.TypeRef;
|
||||||
import io.modelcontextprotocol.json.schema.JsonSchemaValidator;
|
import io.modelcontextprotocol.json.schema.JsonSchemaValidator;
|
||||||
import io.modelcontextprotocol.server.McpStatelessServerFeatures.AsyncResourceTemplateSpecification;
|
import io.modelcontextprotocol.server.McpStatelessServerFeatures.AsyncResourceTemplateSpecification;
|
||||||
import io.modelcontextprotocol.spec.McpError;
|
import io.modelcontextprotocol.spec.McpError;
|
||||||
|
|
@ -28,16 +38,6 @@ import org.slf4j.LoggerFactory;
|
||||||
import reactor.core.publisher.Flux;
|
import reactor.core.publisher.Flux;
|
||||||
import reactor.core.publisher.Mono;
|
import reactor.core.publisher.Mono;
|
||||||
|
|
||||||
import java.time.Duration;
|
|
||||||
import java.util.ArrayList;
|
|
||||||
import java.util.HashMap;
|
|
||||||
import java.util.List;
|
|
||||||
import java.util.Map;
|
|
||||||
import java.util.Optional;
|
|
||||||
import java.util.concurrent.ConcurrentHashMap;
|
|
||||||
import java.util.concurrent.CopyOnWriteArrayList;
|
|
||||||
import java.util.function.BiFunction;
|
|
||||||
|
|
||||||
import static io.modelcontextprotocol.spec.McpError.RESOURCE_NOT_FOUND;
|
import static io.modelcontextprotocol.spec.McpError.RESOURCE_NOT_FOUND;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
@ -293,9 +293,11 @@ public class McpStatelessAsyncServer {
|
||||||
var validation = this.jsonSchemaValidator.validate(outputSchema, result.structuredContent());
|
var validation = this.jsonSchemaValidator.validate(outputSchema, result.structuredContent());
|
||||||
|
|
||||||
if (!validation.valid()) {
|
if (!validation.valid()) {
|
||||||
logger.warn("Tool call result validation failed: {}", validation.errorMessage());
|
String message = "Tool (" + request.name() + ") output validation failed: "
|
||||||
|
+ validation.errorMessage();
|
||||||
|
logger.warn(message);
|
||||||
return CallToolResult.builder()
|
return CallToolResult.builder()
|
||||||
.content(List.of(McpSchema.TextContent.builder(validation.errorMessage()).build()))
|
.content(List.of(McpSchema.TextContent.builder(message).build()))
|
||||||
.isError(true)
|
.isError(true)
|
||||||
.build();
|
.build();
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -42,9 +42,10 @@ public final class ToolInputValidator {
|
||||||
Map<String, Object> args = arguments != null ? arguments : Map.of();
|
Map<String, Object> args = arguments != null ? arguments : Map.of();
|
||||||
var validation = validator.validate(tool.inputSchema(), args);
|
var validation = validator.validate(tool.inputSchema(), args);
|
||||||
if (!validation.valid()) {
|
if (!validation.valid()) {
|
||||||
logger.warn("Tool '{}' input validation failed: {}", tool.name(), validation.errorMessage());
|
String message = "Tool (" + tool.name() + ") input validation failed: " + validation.errorMessage();
|
||||||
|
logger.warn(message);
|
||||||
return CallToolResult.builder()
|
return CallToolResult.builder()
|
||||||
.content(List.of(McpSchema.TextContent.builder(validation.errorMessage()).build()))
|
.content(List.of(McpSchema.TextContent.builder(message).build()))
|
||||||
.isError(true)
|
.isError(true)
|
||||||
.build();
|
.build();
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -76,8 +76,7 @@ public class DefaultJsonSchemaValidator implements JsonSchemaValidator {
|
||||||
// Check if validation passed
|
// Check if validation passed
|
||||||
if (!validationResult.isEmpty()) {
|
if (!validationResult.isEmpty()) {
|
||||||
return ValidationResponse
|
return ValidationResponse
|
||||||
.asInvalid("Validation failed: structuredContent does not match tool outputSchema. "
|
.asInvalid("Validation failed: JSON schema validation errors: " + validationResult);
|
||||||
+ "Validation errors: " + validationResult);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
return ValidationResponse.asValid(jsonStructuredOutput.toString());
|
return ValidationResponse.asValid(jsonStructuredOutput.toString());
|
||||||
|
|
|
||||||
|
|
@ -308,7 +308,7 @@ class DefaultJsonSchemaValidatorTests {
|
||||||
assertFalse(response.valid());
|
assertFalse(response.valid());
|
||||||
assertNotNull(response.errorMessage());
|
assertNotNull(response.errorMessage());
|
||||||
assertTrue(response.errorMessage().contains("Validation failed"));
|
assertTrue(response.errorMessage().contains("Validation failed"));
|
||||||
assertTrue(response.errorMessage().contains("structuredContent does not match tool outputSchema"));
|
assertTrue(response.errorMessage().contains("JSON schema validation errors"));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|
|
||||||
|
|
@ -75,8 +75,7 @@ public class DefaultJsonSchemaValidator implements JsonSchemaValidator {
|
||||||
// Check if validation passed
|
// Check if validation passed
|
||||||
if (!validationResult.isEmpty()) {
|
if (!validationResult.isEmpty()) {
|
||||||
return ValidationResponse
|
return ValidationResponse
|
||||||
.asInvalid("Validation failed: structuredContent does not match tool outputSchema. "
|
.asInvalid("Validation failed: JSON schema validation errors: " + validationResult);
|
||||||
+ "Validation errors: " + validationResult);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
return ValidationResponse.asValid(jsonStructuredOutput.toString());
|
return ValidationResponse.asValid(jsonStructuredOutput.toString());
|
||||||
|
|
|
||||||
|
|
@ -308,7 +308,7 @@ class DefaultJsonSchemaValidatorTests {
|
||||||
assertFalse(response.valid());
|
assertFalse(response.valid());
|
||||||
assertNotNull(response.errorMessage());
|
assertNotNull(response.errorMessage());
|
||||||
assertTrue(response.errorMessage().contains("Validation failed"));
|
assertTrue(response.errorMessage().contains("Validation failed"));
|
||||||
assertTrue(response.errorMessage().contains("structuredContent does not match tool outputSchema"));
|
assertTrue(response.errorMessage().contains("JSON schema validation errors"));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|
|
||||||
|
|
@ -182,6 +182,9 @@ class ToolInputValidationIntegrationTests {
|
||||||
|
|
||||||
assertThat(result.isError()).isTrue();
|
assertThat(result.isError()).isTrue();
|
||||||
String errorMessage = ((TextContent) result.content().get(0)).text();
|
String errorMessage = ((TextContent) result.content().get(0)).text();
|
||||||
|
assertThat(errorMessage).startsWith("Tool (test-tool) input validation failed:");
|
||||||
|
assertThat(errorMessage).containsIgnoringCase("Validation failed");
|
||||||
|
assertThat(errorMessage).containsIgnoringCase("JSON schema validation errors");
|
||||||
assertThat(errorMessage).containsIgnoringCase(expectedErrorSubstring);
|
assertThat(errorMessage).containsIgnoringCase(expectedErrorSubstring);
|
||||||
}
|
}
|
||||||
finally {
|
finally {
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue