Skip to content

Commit 3c901c2

Browse files
committed
mcp: preserve existing Content in SetError
SetError now only populates the Content field when it has not already been populated (len check handles both nil and empty slice). This allows callers to provide a user-friendly message while still recording the underlying error for middleware inspection via GetError. Fixes #858
1 parent 1942036 commit 3c901c2

2 files changed

Lines changed: 60 additions & 3 deletions

File tree

mcp/mcp_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2244,4 +2244,45 @@ func TestToolErrorMiddleware(t *testing.T) {
22442244
}
22452245
}
22462246

2247+
func TestSetErrorPreservesContent(t *testing.T) {
2248+
for _, tt := range []struct {
2249+
name string
2250+
content []Content
2251+
err error
2252+
wantContent string
2253+
}{
2254+
{
2255+
name: "nil content",
2256+
err: errors.New("internal failure"),
2257+
wantContent: "internal failure",
2258+
},
2259+
{
2260+
name: "empty slice content",
2261+
content: []Content{},
2262+
err: errors.New("internal failure"),
2263+
wantContent: "internal failure",
2264+
},
2265+
{
2266+
name: "existing content preserved",
2267+
content: []Content{&TextContent{Text: "user-friendly msg"}},
2268+
err: errors.New("db timeout"),
2269+
wantContent: "user-friendly msg",
2270+
},
2271+
} {
2272+
t.Run(tt.name, func(t *testing.T) {
2273+
res := CallToolResult{Content: tt.content}
2274+
res.SetError(tt.err)
2275+
if !res.IsError {
2276+
t.Fatal("want IsError=true")
2277+
}
2278+
if got := res.Content[0].(*TextContent).Text; got != tt.wantContent {
2279+
t.Errorf("Content text = %q, want %q", got, tt.wantContent)
2280+
}
2281+
if got := res.GetError(); got != tt.err {
2282+
t.Errorf("GetError() = %v, want %v", got, tt.err)
2283+
}
2284+
})
2285+
}
2286+
}
2287+
22472288
var ctrCmpOpts = []cmp.Option{cmp.AllowUnexported(CallToolResult{})}

mcp/protocol.go

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"maps"
1717

1818
internaljson "github.com/modelcontextprotocol/go-sdk/internal/json"
19+
"github.com/modelcontextprotocol/go-sdk/internal/mcpgodebug"
1920
)
2021

2122
// Optional annotations for the client. The client can use annotations to inform
@@ -119,10 +120,25 @@ type CallToolResult struct {
119120
err error
120121
}
121122

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

0 commit comments

Comments
 (0)