mirror of
https://github.com/navidrome/navidrome.git
synced 2025-05-08 06:11:07 +03:00
fix: handle error messages returned in MCP tool response content
Modify callMCPTool helper to check if the text content returned by\nthe MCP server starts with 'handler returned an error:'.\n\nIf it does, log a warning and return agents.ErrNotFound, treating\nthe error signaled by the external tool as if the data was not found.\nAdded test cases to verify this behavior.
This commit is contained in:
parent
8ebefe4065
commit
c548168503
@ -216,12 +216,18 @@ func (a *MCPAgent) callMCPTool(ctx context.Context, toolName string, args any) (
|
|||||||
|
|
||||||
// Process the response
|
// Process the response
|
||||||
if response == nil || len(response.Content) == 0 || response.Content[0].TextContent == nil || response.Content[0].TextContent.Text == "" {
|
if response == nil || len(response.Content) == 0 || response.Content[0].TextContent == nil || response.Content[0].TextContent.Text == "" {
|
||||||
log.Warn(ctx, "MCP tool returned empty or invalid response", "tool", toolName)
|
log.Warn(ctx, "MCP tool returned empty or invalid response structure", "tool", toolName)
|
||||||
return "", agents.ErrNotFound
|
return "", agents.ErrNotFound
|
||||||
}
|
}
|
||||||
|
|
||||||
// Return the text content
|
// Check if the returned text content itself indicates an error from the MCP tool
|
||||||
resultText := response.Content[0].TextContent.Text
|
resultText := response.Content[0].TextContent.Text
|
||||||
|
if strings.HasPrefix(resultText, "handler returned an error:") {
|
||||||
|
log.Warn(ctx, "MCP tool returned an error message in its response", "tool", toolName, "mcpError", resultText)
|
||||||
|
return "", agents.ErrNotFound // Treat MCP tool errors as "not found"
|
||||||
|
}
|
||||||
|
|
||||||
|
// Return the successful text content
|
||||||
log.Debug(ctx, "Received response from MCP agent", "tool", toolName, "length", len(resultText))
|
log.Debug(ctx, "Received response from MCP agent", "tool", toolName, "length", len(resultText))
|
||||||
return resultText, nil
|
return resultText, nil
|
||||||
}
|
}
|
||||||
|
@ -130,6 +130,17 @@ var _ = Describe("MCPAgent", func() {
|
|||||||
Expect(errors.Is(err, io.ErrClosedPipe)).To(BeTrue())
|
Expect(errors.Is(err, io.ErrClosedPipe)).To(BeTrue())
|
||||||
Expect(bio).To(BeEmpty())
|
Expect(bio).To(BeEmpty())
|
||||||
})
|
})
|
||||||
|
|
||||||
|
It("should return ErrNotFound if MCP tool returns an error string", func() {
|
||||||
|
mcpErrorString := "handler returned an error: something went wrong on the server"
|
||||||
|
mockClient.CallToolFunc = func(ctx context.Context, toolName string, args any) (*mcp_client.ToolResponse, error) {
|
||||||
|
return mcp_client.NewToolResponse(mcp_client.NewTextContent(mcpErrorString)), nil
|
||||||
|
}
|
||||||
|
|
||||||
|
bio, err := agent.GetArtistBiography(ctx, "id1", "Artist Name", "mbid1")
|
||||||
|
Expect(err).To(MatchError(agents.ErrNotFound))
|
||||||
|
Expect(bio).To(BeEmpty())
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
Describe("GetArtistURL", func() {
|
Describe("GetArtistURL", func() {
|
||||||
@ -185,5 +196,16 @@ var _ = Describe("MCPAgent", func() {
|
|||||||
Expect(errors.Is(err, io.ErrClosedPipe)).To(BeTrue())
|
Expect(errors.Is(err, io.ErrClosedPipe)).To(BeTrue())
|
||||||
Expect(url).To(BeEmpty())
|
Expect(url).To(BeEmpty())
|
||||||
})
|
})
|
||||||
|
|
||||||
|
It("should return ErrNotFound if MCP tool returns an error string", func() {
|
||||||
|
mcpErrorString := "handler returned an error: could not find url"
|
||||||
|
mockClient.CallToolFunc = func(ctx context.Context, toolName string, args any) (*mcp_client.ToolResponse, error) {
|
||||||
|
return mcp_client.NewToolResponse(mcp_client.NewTextContent(mcpErrorString)), nil
|
||||||
|
}
|
||||||
|
|
||||||
|
url, err := agent.GetArtistURL(ctx, "id2", "Another Artist", "mbid2")
|
||||||
|
Expect(err).To(MatchError(agents.ErrNotFound))
|
||||||
|
Expect(url).To(BeEmpty())
|
||||||
|
})
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
Loading…
x
Reference in New Issue
Block a user