From 6f6e196c27aa0b7aae022aebfcc8abb58eb1249e Mon Sep 17 00:00:00 2001
From: Rob Findley <rfindley@golang.org>
Date: Wed, 10 Dec 2025 16:50:39 -0500
Subject: [PATCH] mcp: fix stray log.Printf in notifySessions

I encountered these logs, and they were distracting and noisy. We
shouldn't log to stderr unless configured by the user. Thread through
the logger.

Also, use ensureLogger in the streamable client to simplify call sites.

Updates #708
Updates #684
---
 mcp/client.go     |  5 ++++-
 mcp/server.go     |  4 ++--
 mcp/shared.go     |  8 ++++----
 mcp/streamable.go | 10 +++-------
 4 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/mcp/client.go b/mcp/client.go
index 7349ba9..b862c79 100644
--- a/mcp/client.go
+++ b/mcp/client.go
@@ -10,6 +10,7 @@ import (
 	"errors"
 	"fmt"
 	"iter"
+	"log/slog"
 	"slices"
 	"strings"
 	"sync"
@@ -26,6 +27,7 @@ import (
 type Client struct {
 	impl                    *Implementation
 	opts                    ClientOptions
+	logger                  *slog.Logger // TODO: file proposal to export this
 	mu                      sync.Mutex
 	roots                   *featureSet[*Root]
 	sessions                []*ClientSession
@@ -46,6 +48,7 @@ func NewClient(impl *Implementation, opts *ClientOptions) *Client {
 	}
 	c := &Client{
 		impl:                    impl,
+		logger:                  ensureLogger(nil), // ensure we have a logger
 		roots:                   newFeatureSet(func(r *Root) string { return r.URI }),
 		sendingMethodHandler_:   defaultSendingMethodHandler,
 		receivingMethodHandler_: defaultReceivingMethodHandler[*ClientSession],
@@ -341,7 +344,7 @@ func changeAndNotify[P Params](c *Client, notification string, params P, change
 		sessions = slices.Clone(c.sessions)
 	}
 	c.mu.Unlock()
-	notifySessions(sessions, notification, params)
+	notifySessions(sessions, notification, params, c.logger)
 }
 
 func (c *Client) listRoots(_ context.Context, req *ListRootsRequest) (*ListRootsResult, error) {
diff --git a/mcp/server.go b/mcp/server.go
index d431722..5f5a4a5 100644
--- a/mcp/server.go
+++ b/mcp/server.go
@@ -508,7 +508,7 @@ func (s *Server) changeAndNotify(notification string, params Params, change func
 		sessions = slices.Clone(s.sessions)
 	}
 	s.mu.Unlock()
-	notifySessions(sessions, notification, params)
+	notifySessions(sessions, notification, params, s.opts.Logger)
 }
 
 // Sessions returns an iterator that yields the current set of server sessions.
@@ -708,7 +708,7 @@ func (s *Server) ResourceUpdated(ctx context.Context, params *ResourceUpdatedNot
 	subscribedSessions := s.resourceSubscriptions[params.URI]
 	sessions := slices.Collect(maps.Keys(subscribedSessions))
 	s.mu.Unlock()
-	notifySessions(sessions, notificationResourceUpdated, params)
+	notifySessions(sessions, notificationResourceUpdated, params, s.opts.Logger)
 	s.opts.Logger.Info("resource updated notification sent", "uri", params.URI, "subscriber_count", len(sessions))
 	return nil
 }
diff --git a/mcp/shared.go b/mcp/shared.go
index f19236f..c2e50d9 100644
--- a/mcp/shared.go
+++ b/mcp/shared.go
@@ -15,7 +15,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"log"
+	"log/slog"
 	"net/http"
 	"reflect"
 	"slices"
@@ -381,7 +381,8 @@ const (
 
 // notifySessions calls Notify on all the sessions.
 // Should be called on a copy of the peer sessions.
-func notifySessions[S Session, P Params](sessions []S, method string, params P) {
+// The logger must be non-nil.
+func notifySessions[S Session, P Params](sessions []S, method string, params P, logger *slog.Logger) {
 	if sessions == nil {
 		return
 	}
@@ -394,8 +395,7 @@ func notifySessions[S Session, P Params](sessions []S, method string, params P)
 	for _, s := range sessions {
 		req := newRequest(s, params)
 		if err := handleNotify(ctx, method, req); err != nil {
-			// TODO(#218): surface this error better
-			log.Printf("calling %s: %v", method, err)
+			logger.Warn(fmt.Sprintf("calling %s: %v", method, err))
 		}
 	}
 }
diff --git a/mcp/streamable.go b/mcp/streamable.go
index a24d96e..d6452b2 100644
--- a/mcp/streamable.go
+++ b/mcp/streamable.go
@@ -1453,7 +1453,7 @@ func (t *StreamableClientTransport) Connect(ctx context.Context) (Connection, er
 		done:       make(chan struct{}),
 		maxRetries: maxRetries,
 		strict:     t.strict,
-		logger:     t.logger,
+		logger:     ensureLogger(t.logger), // must be non-nil for safe logging
 		ctx:        connCtx,
 		cancel:     cancel,
 		failed:     make(chan struct{}),
@@ -1555,9 +1555,7 @@ func (c *streamableClientConn) connectStandaloneSSE() {
 		// stream.
 		//
 		// Treat this like MethodNotAllowed in non-strict mode.
-		if c.logger != nil {
-			c.logger.Warn(fmt.Sprintf("got %d instead of 405 for standalone SSE stream", resp.StatusCode))
-		}
+		c.logger.Warn(fmt.Sprintf("got %d instead of 405 for standalone SSE stream", resp.StatusCode))
 		resp.Body.Close()
 		return
 	}
@@ -1686,9 +1684,7 @@ func (c *streamableClientConn) Write(ctx context.Context, msg jsonrpc.Message) e
 			// Some servers return 200, even with an empty json body.
 			//
 			// In strict mode, return an error to the caller.
-			if c.logger != nil {
-				c.logger.Warn(errMsg)
-			}
+			c.logger.Warn(errMsg)
 			if c.strict {
 				return errors.New(errMsg)
 			}