From 8cb01cc89e64cb10a8fc8ba42ebbe6980b856b35 Mon Sep 17 00:00:00 2001
From: Ravish <rgupt3888@gmail.com>
Date: Fri, 27 Mar 2026 15:29:34 -0700
Subject: [PATCH] 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
---
 mcp/mcp_test.go | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
 mcp/protocol.go | 11 ++++++++---
 2 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/mcp/mcp_test.go b/mcp/mcp_test.go
index 86c05502..838a39a9 100644
--- a/mcp/mcp_test.go
+++ b/mcp/mcp_test.go
@@ -2244,4 +2244,54 @@ func TestToolErrorMiddleware(t *testing.T) {
 	}
 }
 
+func TestSetErrorPreservesContent(t *testing.T) {
+	t.Run("empty content is populated", func(t *testing.T) {
+		var res CallToolResult
+		res.SetError(errors.New("internal failure"))
+		if !res.IsError {
+			t.Fatal("want IsError=true")
+		}
+		text := res.Content[0].(*TextContent).Text
+		if text != "internal failure" {
+			t.Errorf("got %q, want %q", text, "internal failure")
+		}
+		if res.GetError() == nil || res.GetError().Error() != "internal failure" {
+			t.Errorf("GetError() = %v, want 'internal failure'", res.GetError())
+		}
+	})
+
+	t.Run("empty slice content is populated", func(t *testing.T) {
+		res := CallToolResult{
+			Content: []Content{},
+		}
+		res.SetError(errors.New("internal failure"))
+		if !res.IsError {
+			t.Fatal("want IsError=true")
+		}
+		text := res.Content[0].(*TextContent).Text
+		if text != "internal failure" {
+			t.Errorf("got %q, want %q", text, "internal failure")
+		}
+	})
+
+	t.Run("existing content is preserved", func(t *testing.T) {
+		res := CallToolResult{
+			Content: []Content{&TextContent{Text: "Something went wrong, please try again."}},
+		}
+		res.SetError(errors.New("db timeout on query SELECT * FROM users"))
+		if !res.IsError {
+			t.Fatal("want IsError=true")
+		}
+		// User-facing content should be preserved.
+		text := res.Content[0].(*TextContent).Text
+		if text != "Something went wrong, please try again." {
+			t.Errorf("Content was overwritten: got %q", text)
+		}
+		// Internal error should still be accessible via GetError.
+		if res.GetError() == nil || res.GetError().Error() != "db timeout on query SELECT * FROM users" {
+			t.Errorf("GetError() = %v, want 'db timeout on query SELECT * FROM users'", res.GetError())
+		}
+	})
+}
+
 var ctrCmpOpts = []cmp.Option{cmp.AllowUnexported(CallToolResult{})}
diff --git a/mcp/protocol.go b/mcp/protocol.go
index 837ce784..d37e37d3 100644
--- a/mcp/protocol.go
+++ b/mcp/protocol.go
@@ -119,10 +119,15 @@ type CallToolResult struct {
 	err error
 }
 
-// SetError sets the error for the tool result and populates the Content field
-// with the error text. It also sets IsError to true.
+// SetError sets the error for the tool result and sets IsError to true.
+// If Content has not already been populated, it is set to the error text.
+// If Content has already been populated, it is left unchanged, allowing callers
+// to provide a user-friendly message while still recording the underlying error
+// for inspection via [GetError] in server middleware.
 func (r *CallToolResult) SetError(err error) {
-	r.Content = []Content{&TextContent{Text: err.Error()}}
+	if len(r.Content) == 0 {
+		r.Content = []Content{&TextContent{Text: err.Error()}}
+	}
 	r.IsError = true
 	r.err = err
 }