Skip to content

Produce JSONRPCError for error response #519

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+169 −57
Open

Produce JSONRPCError for error response #519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions .../java/io/modelcontextprotocol/server/transport/HttpServletSseServerTransportProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
*
* @author Christian Tzolov
* @author Alexandros Pappas
* @author Yanming Zhou
* @see McpServerTransportProvider
* @see HttpServlet
*/
Expand Down Expand Up @@ -278,7 +279,9 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
response.setContentType(APPLICATION_JSON);
response.setCharacterEncoding(UTF_8);
response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
String jsonError = jsonMapper.writeValueAsString(new McpError("Session ID missing in message endpoint"));
String jsonError = jsonMapper.writeValueAsString(McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST)
.message("Session ID missing in message endpoint")
.build());
PrintWriter writer = response.getWriter();
writer.write(jsonError);
writer.flush();
Expand All @@ -291,7 +294,9 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
response.setContentType(APPLICATION_JSON);
response.setCharacterEncoding(UTF_8);
response.setStatus(HttpServletResponse.SC_NOT_FOUND);
String jsonError = jsonMapper.writeValueAsString(new McpError("Session not found: " + sessionId));
String jsonError = jsonMapper.writeValueAsString(McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST)
.message("Session not found: " + sessionId)
.build());
PrintWriter writer = response.getWriter();
writer.write(jsonError);
writer.flush();
Expand All @@ -318,7 +323,9 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
catch (Exception e) {
logger.error("Error processing message: {}", e.getMessage());
try {
McpError mcpError = new McpError(e.getMessage());
McpError mcpError = McpError.builder(McpSchema.ErrorCodes.INTERNAL_ERROR)
.message(e.getMessage())
.build();
response.setContentType(APPLICATION_JSON);
response.setCharacterEncoding(UTF_8);
response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
Expand Down
24 changes: 18 additions & 6 deletions ...in/java/io/modelcontextprotocol/server/transport/HttpServletStatelessServerTransport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
*
* @author Christian Tzolov
* @author Dariusz Jędrzejczyk
* @author Yanming Zhou
*/
@WebServlet(asyncSupported = true)
public class HttpServletStatelessServerTransport extends HttpServlet implements McpStatelessServerTransport {
Expand Down Expand Up @@ -127,7 +128,9 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
String accept = request.getHeader(ACCEPT);
if (accept == null || !(accept.contains(APPLICATION_JSON) && accept.contains(TEXT_EVENT_STREAM))) {
this.responseError(response, HttpServletResponse.SC_BAD_REQUEST,
new McpError("Both application/json and text/event-stream required in Accept header"));
McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST)
.message("Invalid Accept headers. Expected application/json and text/event-stream")
.build());
return;
}

Expand Down Expand Up @@ -160,7 +163,9 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
catch (Exception e) {
logger.error("Failed to handle request: {}", e.getMessage());
this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
new McpError("Failed to handle request: " + e.getMessage()));
McpError.builder(McpSchema.ErrorCodes.INTERNAL_ERROR)
.message("Failed to handle request: " + e.getMessage())
.build());
}
}
else if (message instanceof McpSchema.JSONRPCNotification jsonrpcNotification) {
Expand All @@ -173,22 +178,29 @@ else if (message instanceof McpSchema.JSONRPCNotification jsonrpcNotification) {
catch (Exception e) {
logger.error("Failed to handle notification: {}", e.getMessage());
this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
new McpError("Failed to handle notification: " + e.getMessage()));
McpError.builder(McpSchema.ErrorCodes.INTERNAL_ERROR)
.message("Failed to handle notification: " + e.getMessage())
.build());
}
}
else {
this.responseError(response, HttpServletResponse.SC_BAD_REQUEST,
new McpError("The server accepts either requests or notifications"));
McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST)
.message("The server accepts either requests or notifications")
.build());
}
}
catch (IllegalArgumentException | IOException e) {
logger.error("Failed to deserialize message: {}", e.getMessage());
this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, new McpError("Invalid message format"));
this.responseError(response, HttpServletResponse.SC_BAD_REQUEST,
McpError.builder(McpSchema.ErrorCodes.PARSE_ERROR).message("Invalid message format").build());
}
catch (Exception e) {
logger.error("Unexpected error handling message: {}", e.getMessage());
this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
new McpError("Unexpected error: " + e.getMessage()));
McpError.builder(McpSchema.ErrorCodes.INTERNAL_ERROR)
.message("Unexpected error: " + e.getMessage())
.build());
}
}

Expand Down
36 changes: 25 additions & 11 deletions ...o/modelcontextprotocol/server/transport/HttpServletStreamableServerTransportProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
* @author Zachary German
* @author Christian Tzolov
* @author Dariusz Jędrzejczyk
* @author Yanming Zhou
* @see McpStreamableServerTransportProvider
* @see HttpServlet
*/
Expand Down Expand Up @@ -261,7 +262,8 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response)

if (!badRequestErrors.isEmpty()) {
String combinedMessage = String.join("; ", badRequestErrors);
this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, new McpError(combinedMessage));
this.responseError(response, HttpServletResponse.SC_BAD_REQUEST,
McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST).message(combinedMessage).build());
return;
}

Expand Down Expand Up @@ -400,7 +402,8 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
&& jsonrpcRequest.method().equals(McpSchema.METHOD_INITIALIZE)) {
if (!badRequestErrors.isEmpty()) {
String combinedMessage = String.join("; ", badRequestErrors);
this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, new McpError(combinedMessage));
this.responseError(response, HttpServletResponse.SC_BAD_REQUEST,
McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST).message(combinedMessage).build());
return;
}

Expand Down Expand Up @@ -430,7 +433,9 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
catch (Exception e) {
logger.error("Failed to initialize session: {}", e.getMessage());
this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
new McpError("Failed to initialize session: " + e.getMessage()));
McpError.builder(McpSchema.ErrorCodes.INTERNAL_ERROR)
.message("Failed to initialize session: " + e.getMessage())
.build());
return;
}
}
Expand All @@ -443,15 +448,18 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)

if (!badRequestErrors.isEmpty()) {
String combinedMessage = String.join("; ", badRequestErrors);
this.responseError(response, HttpServletResponse.SC_BAD_REQUEST, new McpError(combinedMessage));
this.responseError(response, HttpServletResponse.SC_BAD_REQUEST,
McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST).message(combinedMessage).build());
return;
}

McpStreamableServerSession session = this.sessions.get(sessionId);

if (session == null) {
this.responseError(response, HttpServletResponse.SC_NOT_FOUND,
new McpError("Session not found: " + sessionId));
McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST)
.message("Session not found: " + sessionId)
.build());
return;
}

Expand Down Expand Up @@ -492,20 +500,24 @@ else if (message instanceof McpSchema.JSONRPCRequest jsonrpcRequest) {
}
}
else {
this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
new McpError("Unknown message type"));
this.responseError(response, HttpServletResponse.SC_BAD_REQUEST,
McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST).message("Unknown message type").build());
}
}
catch (IllegalArgumentException | IOException e) {
logger.error("Failed to deserialize message: {}", e.getMessage());
this.responseError(response, HttpServletResponse.SC_BAD_REQUEST,
new McpError("Invalid message format: " + e.getMessage()));
McpError.builder(McpSchema.ErrorCodes.PARSE_ERROR)
.message("Invalid message format: " + e.getMessage())
.build());
}
catch (Exception e) {
logger.error("Error handling message: {}", e.getMessage());
try {
this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
new McpError("Error processing message: " + e.getMessage()));
McpError.builder(McpSchema.ErrorCodes.INTERNAL_ERROR)
.message("Error processing message: " + e.getMessage())
.build());
}
catch (IOException ex) {
logger.error(FAILED_TO_SEND_ERROR_RESPONSE, ex.getMessage());
Expand Down Expand Up @@ -545,7 +557,9 @@ protected void doDelete(HttpServletRequest request, HttpServletResponse response

if (request.getHeader(HttpHeaders.MCP_SESSION_ID) == null) {
this.responseError(response, HttpServletResponse.SC_BAD_REQUEST,
new McpError("Session ID required in mcp-session-id header"));
McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST)
.message("Session ID required in mcp-session-id header")
.build());
return;
}

Expand All @@ -566,7 +580,7 @@ protected void doDelete(HttpServletRequest request, HttpServletResponse response
logger.error("Failed to delete session {}: {}", sessionId, e.getMessage());
try {
this.responseError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
new McpError(e.getMessage()));
McpError.builder(McpSchema.ErrorCodes.INTERNAL_ERROR).message(e.getMessage()).build());
}
catch (IOException ex) {
logger.error(FAILED_TO_SEND_ERROR_RESPONSE, ex.getMessage());
Expand Down
5 changes: 4 additions & 1 deletion mcp-core/src/main/java/io/modelcontextprotocol/spec/McpError.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

package io.modelcontextprotocol.spec;

import com.fasterxml.jackson.annotation.JsonValue;
import io.modelcontextprotocol.spec.McpSchema.JSONRPCResponse.JSONRPCError;
import io.modelcontextprotocol.util.Assert;

Expand All @@ -20,7 +21,8 @@ public class McpError extends RuntimeException {
public static final Function<String, McpError> RESOURCE_NOT_FOUND = resourceUri -> new McpError(new JSONRPCError(
McpSchema.ErrorCodes.RESOURCE_NOT_FOUND, "Resource not found", Map.of("uri", resourceUri)));

private JSONRPCError jsonRpcError;
@JsonValue
private final JSONRPCError jsonRpcError;

public McpError(JSONRPCError jsonRpcError) {
super(jsonRpcError.message());
Expand All @@ -30,6 +32,7 @@ public McpError(JSONRPCError jsonRpcError) {
@Deprecated
public McpError(Object error) {
super(error.toString());
this.jsonRpcError = new JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR, error.toString(), null);
}

public JSONRPCError getJsonRpcError() {
Expand Down
19 changes: 15 additions & 4 deletions ...main/java/io/modelcontextprotocol/server/transport/WebFluxSseServerTransportProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
* @author Christian Tzolov
* @author Alexandros Pappas
* @author Dariusz Jędrzejczyk
* @author Yanming Zhou
* @see McpServerTransport
* @see ServerSentEvent
*/
Expand Down Expand Up @@ -318,14 +319,19 @@ private Mono<ServerResponse> handleMessage(ServerRequest request) {
}

if (request.queryParam("sessionId").isEmpty()) {
return ServerResponse.badRequest().bodyValue(new McpError("Session ID missing in message endpoint"));
return ServerResponse.badRequest()
.bodyValue(McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST)
.message("Session ID missing in message endpoint")
.build());
}

McpServerSession session = sessions.get(request.queryParam("sessionId").get());

if (session == null) {
return ServerResponse.status(HttpStatus.NOT_FOUND)
.bodyValue(new McpError("Session not found: " + request.queryParam("sessionId").get()));
.bodyValue(McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST)
.message("Session not found: " + request.queryParam("sessionId").get())
.build());
}

McpTransportContext transportContext = this.contextExtractor.extract(request);
Expand All @@ -339,12 +345,17 @@ private Mono<ServerResponse> handleMessage(ServerRequest request) {
// - the error is signalled on the SSE connection
// return ServerResponse.ok().build();
return ServerResponse.status(HttpStatus.INTERNAL_SERVER_ERROR)
.bodyValue(new McpError(error.getMessage()));
.bodyValue(McpError.builder(McpSchema.ErrorCodes.INTERNAL_ERROR)
.message(error.getMessage())
.build());
});
}
catch (IllegalArgumentException | IOException e) {
logger.error("Failed to deserialize message: {}", e.getMessage());
return ServerResponse.badRequest().bodyValue(new McpError("Invalid message format"));
return ServerResponse.badRequest()
.bodyValue(McpError.builder(McpSchema.ErrorCodes.PARSE_ERROR)
.message("Invalid message format")
.build());
}
}).contextWrite(ctx -> ctx.put(McpTransportContext.KEY, transportContext));
}
Expand Down
15 changes: 12 additions & 3 deletions ...c/main/java/io/modelcontextprotocol/server/transport/WebFluxStatelessServerTransport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
* Implementation of a WebFlux based {@link McpStatelessServerTransport}.
*
* @author Dariusz Jędrzejczyk
* @author Yanming Zhou
*/
public class WebFluxStatelessServerTransport implements McpStatelessServerTransport {

Expand Down Expand Up @@ -101,7 +102,10 @@ private Mono<ServerResponse> handlePost(ServerRequest request) {
List<MediaType> acceptHeaders = request.headers().asHttpHeaders().getAccept();
if (!(acceptHeaders.contains(MediaType.APPLICATION_JSON)
&& acceptHeaders.contains(MediaType.TEXT_EVENT_STREAM))) {
return ServerResponse.badRequest().build();
return ServerResponse.badRequest()
.bodyValue(McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST)
.message("Invalid Accept headers. Expected application/json and text/event-stream")
.build());
}

return request.bodyToMono(String.class).<ServerResponse>flatMap(body -> {
Expand All @@ -127,12 +131,17 @@ else if (message instanceof McpSchema.JSONRPCNotification jsonrpcNotification) {
}
else {
return ServerResponse.badRequest()
.bodyValue(new McpError("The server accepts either requests or notifications"));
.bodyValue(McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST)
.message("The server accepts either requests or notifications")
.build());
}
}
catch (IllegalArgumentException | IOException e) {
logger.error("Failed to deserialize message: {}", e.getMessage());
return ServerResponse.badRequest().bodyValue(new McpError("Invalid message format"));
return ServerResponse.badRequest()
.bodyValue(McpError.builder(McpSchema.ErrorCodes.PARSE_ERROR)
.message("Invalid message format")
.build());
}
}).contextWrite(ctx -> ctx.put(McpTransportContext.KEY, transportContext));
}
Expand Down
Loading