Skip to content

Commit 34a19df

Browse files
authored
mcp: preserve existing Content in SetError (#864)
SetError now only populates the Content field when it has not already been populated. This allows callers to provide a user-friendly message while still recording the underlying error for middleware inspection via GetError. The len check (rather than nil check) ensures both nil and empty-slice Content are treated as "not yet populated." Fixes #858
1 parent 27f29c1 commit 34a19df

File tree

2 files changed

+60
-3
lines changed

2 files changed

+60
-3
lines changed

mcp/mcp_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2228,4 +2228,45 @@ func TestToolErrorMiddleware(t *testing.T) {
22282228
}
22292229
}
22302230

2231+
func TestSetErrorPreservesContent(t *testing.T) {
2232+
for _, tt := range []struct {
2233+
name string
2234+
content []Content
2235+
err error
2236+
wantContent string
2237+
}{
2238+
{
2239+
name: "nil content",
2240+
err: errors.New("internal failure"),
2241+
wantContent: "internal failure",
2242+
},
2243+
{
2244+
name: "empty slice content",
2245+
content: []Content{},
2246+
err: errors.New("internal failure"),
2247+
wantContent: "internal failure",
2248+
},
2249+
{
2250+
name: "existing content preserved",
2251+
content: []Content{&TextContent{Text: "user-friendly msg"}},
2252+
err: errors.New("db timeout"),
2253+
wantContent: "user-friendly msg",
2254+
},
2255+
} {
2256+
t.Run(tt.name, func(t *testing.T) {
2257+
res := CallToolResult{Content: tt.content}
2258+
res.SetError(tt.err)
2259+
if !res.IsError {
2260+
t.Fatal("want IsError=true")
2261+
}
2262+
if got := res.Content[0].(*TextContent).Text; got != tt.wantContent {
2263+
t.Errorf("Content text = %q, want %q", got, tt.wantContent)
2264+
}
2265+
if got := res.GetError(); got != tt.err {
2266+
t.Errorf("GetError() = %v, want %v", got, tt.err)
2267+
}
2268+
})
2269+
}
2270+
}
2271+
22312272
var ctrCmpOpts = []cmp.Option{cmp.AllowUnexported(CallToolResult{})}

mcp/protocol.go

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"maps"
1111

1212
internaljson "github.com/modelcontextprotocol/go-sdk/internal/json"
13+
"github.com/modelcontextprotocol/go-sdk/internal/mcpgodebug"
1314
)
1415

1516
// Optional annotations for the client. The client can use annotations to inform
@@ -113,10 +114,25 @@ type CallToolResult struct {
113114
err error
114115
}
115116

116-
// SetError sets the error for the tool result and populates the Content field
117-
// with the error text. It also sets IsError to true.
117+
// seterroroverwrite is a compatibility parameter that restores the pre-1.6.0
118+
// behavior of [CallToolResult.SetError], where Content was always overwritten
119+
// with the error text. See the documentation for the mcpgodebug package for
120+
// instructions on how to enable it.
121+
// The option will be removed in the 1.8.0 version of the SDK.
122+
var seterroroverwrite = mcpgodebug.Value("seterroroverwrite")
123+
124+
// SetError sets the error for the tool result and sets IsError to true.
125+
// If Content has not already been populated, it is set to the error text.
126+
// If Content has already been populated, it is left unchanged, allowing callers
127+
// to provide a user-friendly message while still recording the underlying error
128+
// for inspection via [GetError] in server middleware.
129+
//
130+
// To restore the previous behavior where Content was always overwritten,
131+
// set MCPGODEBUG=seterroroverwrite=1.
118132
func (r *CallToolResult) SetError(err error) {
119-
r.Content = []Content{&TextContent{Text: err.Error()}}
133+
if len(r.Content) == 0 || seterroroverwrite == "1" {
134+
r.Content = []Content{&TextContent{Text: err.Error()}}
135+
}
120136
r.IsError = true
121137
r.err = err
122138
}

0 commit comments

Comments
 (0)