client: Honor IsError and extract JSON from tool text
CI / Test and build (pull_request) Successful in 14s
CI / Test and build (pull_request) Successful in 14s
Fold Jerry dump into toolJSON: TrimSpace IsError text, concat JSON via rawJSON, and unit tests mirroring IsError=true / StructuredContent=null / Text=`end must be after start` / GetError=nil.
This commit is contained in:
+20
-12
@@ -100,18 +100,18 @@ func toolJSON(res *mcp.CallToolResult) (json.RawMessage, error) {
|
||||
if res == nil {
|
||||
return nil, fmt.Errorf("empty tool result")
|
||||
}
|
||||
// GetError is only set by SetError on the server; the err field is not marshaled to clients.
|
||||
// GetError is set by SetError on the server; the err field is not marshaled to clients.
|
||||
if err := res.GetError(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
texts, types := collectTextAndTypes(res)
|
||||
concat := strings.Join(texts, "")
|
||||
// IsError is the client-visible soft-fail flag; error text lives in Content.
|
||||
if res.IsError {
|
||||
if concat == "" {
|
||||
return nil, fmt.Errorf("tool error")
|
||||
msg := strings.TrimSpace(concat)
|
||||
if msg == "" {
|
||||
msg = "tool error"
|
||||
}
|
||||
return nil, fmt.Errorf("%s", concat)
|
||||
return nil, fmt.Errorf("%s", msg)
|
||||
}
|
||||
if res.StructuredContent != nil {
|
||||
b, err := json.Marshal(res.StructuredContent)
|
||||
@@ -121,16 +121,16 @@ func toolJSON(res *mcp.CallToolResult) (json.RawMessage, error) {
|
||||
return json.RawMessage(b), nil
|
||||
}
|
||||
for _, t := range texts {
|
||||
if json.Valid([]byte(t)) {
|
||||
return json.RawMessage(t), nil
|
||||
if raw, ok := rawJSON(t); ok {
|
||||
return raw, nil
|
||||
}
|
||||
}
|
||||
if raw, ok := rawJSON(concat); ok {
|
||||
return raw, nil
|
||||
}
|
||||
if concat == "" {
|
||||
return json.RawMessage(`{}`), nil
|
||||
}
|
||||
if json.Valid([]byte(concat)) {
|
||||
return json.RawMessage(concat), nil
|
||||
}
|
||||
if raw := extractFirstJSON(concat); raw != nil {
|
||||
return raw, nil
|
||||
}
|
||||
@@ -138,8 +138,16 @@ func toolJSON(res *mcp.CallToolResult) (json.RawMessage, error) {
|
||||
if res.StructuredContent != nil {
|
||||
structured = "present"
|
||||
}
|
||||
return nil, fmt.Errorf("non-json tool result (isError=%v types=[%s] structured=%s textLen=%d prefix=%q)",
|
||||
res.IsError, strings.Join(types, " "), structured, utf8.RuneCountInString(concat), runePrefix(concat, 80))
|
||||
return nil, fmt.Errorf("non-json tool result (isError=%v types=%v structured=%s textLen=%d prefix=%q)",
|
||||
res.IsError, types, structured, utf8.RuneCountInString(concat), runePrefix(concat, 80))
|
||||
}
|
||||
|
||||
func rawJSON(s string) (json.RawMessage, bool) {
|
||||
b := []byte(s)
|
||||
if !json.Valid(b) {
|
||||
return nil, false
|
||||
}
|
||||
return json.RawMessage(b), true
|
||||
}
|
||||
|
||||
func collectTextAndTypes(res *mcp.CallToolResult) (texts, types []string) {
|
||||
|
||||
+44
-8
@@ -46,7 +46,7 @@ func TestToolJSON(t *testing.T) {
|
||||
{
|
||||
name: "wrapped JSON in prose",
|
||||
res: &mcp.CallToolResult{
|
||||
Content: []mcp.Content{&mcp.TextContent{Text: `here is data: {"symbol":"MU"} thanks`}},
|
||||
Content: []mcp.Content{&mcp.TextContent{Text: `here: {"symbol":"MU"} ok`}},
|
||||
},
|
||||
want: `{"symbol":"MU"}`,
|
||||
},
|
||||
@@ -60,6 +60,16 @@ func TestToolJSON(t *testing.T) {
|
||||
},
|
||||
want: `{"symbol":"MU"}`,
|
||||
},
|
||||
{
|
||||
name: "concatenated text JSON",
|
||||
res: &mcp.CallToolResult{
|
||||
Content: []mcp.Content{
|
||||
&mcp.TextContent{Text: `{"symbol":`},
|
||||
&mcp.TextContent{Text: `"MU"}`},
|
||||
},
|
||||
},
|
||||
want: `{"symbol":"MU"}`,
|
||||
},
|
||||
{
|
||||
name: "wrapped JSON array",
|
||||
res: &mcp.CallToolResult{
|
||||
@@ -110,18 +120,38 @@ func TestToolJSON_getError(t *testing.T) {
|
||||
|
||||
func TestToolJSON_isErrorText(t *testing.T) {
|
||||
t.Parallel()
|
||||
// Jerry dump: IsError=true, StructuredContent=null, Text=`end must be after start`, GetError=nil.
|
||||
res := &mcp.CallToolResult{
|
||||
IsError: true,
|
||||
Content: []mcp.Content{&mcp.TextContent{Text: "historicals unavailable"}},
|
||||
StructuredContent: nil,
|
||||
Content: []mcp.Content{&mcp.TextContent{Text: "end must be after start"}},
|
||||
}
|
||||
if res.GetError() != nil {
|
||||
t.Fatal("GetError should be nil when IsError is set via field")
|
||||
t.Fatal("GetError should be nil on a client-shaped result")
|
||||
}
|
||||
_, err := toolJSON(res)
|
||||
if err == nil {
|
||||
t.Fatal("expected error")
|
||||
}
|
||||
if err.Error() != "historicals unavailable" {
|
||||
if err.Error() != "end must be after start" {
|
||||
t.Fatalf("%v", err)
|
||||
}
|
||||
if strings.Contains(err.Error(), "non-json tool result") {
|
||||
t.Fatalf("got non-json wrapping: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolJSON_isErrorOverStructured(t *testing.T) {
|
||||
t.Parallel()
|
||||
_, err := toolJSON(&mcp.CallToolResult{
|
||||
IsError: true,
|
||||
StructuredContent: map[string]any{"symbol": "MU"},
|
||||
Content: []mcp.Content{&mcp.TextContent{Text: "end must be after start"}},
|
||||
})
|
||||
if err == nil {
|
||||
t.Fatal("expected error")
|
||||
}
|
||||
if err.Error() != "end must be after start" {
|
||||
t.Fatalf("%v", err)
|
||||
}
|
||||
if strings.Contains(err.Error(), "non-json tool result") {
|
||||
@@ -131,7 +161,12 @@ func TestToolJSON_isErrorText(t *testing.T) {
|
||||
|
||||
func TestToolJSON_isErrorEmpty(t *testing.T) {
|
||||
t.Parallel()
|
||||
_, err := toolJSON(&mcp.CallToolResult{IsError: true})
|
||||
tests := []*mcp.CallToolResult{
|
||||
{IsError: true},
|
||||
{IsError: true, Content: []mcp.Content{&mcp.TextContent{Text: " \n"}}},
|
||||
}
|
||||
for _, res := range tests {
|
||||
_, err := toolJSON(res)
|
||||
if err == nil {
|
||||
t.Fatal("expected error")
|
||||
}
|
||||
@@ -142,6 +177,7 @@ func TestToolJSON_isErrorEmpty(t *testing.T) {
|
||||
t.Fatalf("got non-json wrapping: %v", err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolJSON_nonJSONText(t *testing.T) {
|
||||
t.Parallel()
|
||||
@@ -152,10 +188,10 @@ func TestToolJSON_nonJSONText(t *testing.T) {
|
||||
t.Fatal("expected error")
|
||||
}
|
||||
msg := err.Error()
|
||||
if !strings.Contains(msg, "non-json") {
|
||||
t.Fatalf("missing non-json: %v", err)
|
||||
if !strings.Contains(msg, "non-json tool result") {
|
||||
t.Fatalf("missing non-json tool result: %v", err)
|
||||
}
|
||||
for _, want := range []string{`isError=false`, `types=[text]`, `structured=nil`, `prefix="not json"`} {
|
||||
for _, want := range []string{`isError=false`, `types=[text]`, `structured=nil`, `textLen=8`, `prefix="not json"`} {
|
||||
if !strings.Contains(msg, want) {
|
||||
t.Fatalf("missing %q in %v", want, err)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user