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
This commit is contained in:
Rob Findley 2025-12-10 16:50:39 -05:00 • committed by Robert Findley
commit 6f6e196c27
4 changed files with 13 additions and 14 deletions

View file

@ -10,6 +10,7 @@ import (
"errors" "errors"
"fmt" "fmt"
"iter" "iter"
"log/slog"
"slices" "slices"
"strings" "strings"
"sync" "sync"
@ -26,6 +27,7 @@ import (
type Client struct { type Client struct {
impl *Implementation impl *Implementation
opts ClientOptions opts ClientOptions
logger *slog.Logger // TODO: file proposal to export this
mu sync.Mutex mu sync.Mutex
roots *featureSet[*Root] roots *featureSet[*Root]
sessions []*ClientSession sessions []*ClientSession
@ -46,6 +48,7 @@ func NewClient(impl *Implementation, opts *ClientOptions) *Client {
} }
c := &Client{ c := &Client{
impl: impl, impl: impl,
logger: ensureLogger(nil), // ensure we have a logger
roots: newFeatureSet(func(r *Root) string { return r.URI }), roots: newFeatureSet(func(r *Root) string { return r.URI }),
sendingMethodHandler_: defaultSendingMethodHandler, sendingMethodHandler_: defaultSendingMethodHandler,
receivingMethodHandler_: defaultReceivingMethodHandler[*ClientSession], receivingMethodHandler_: defaultReceivingMethodHandler[*ClientSession],
@ -341,7 +344,7 @@ func changeAndNotify[P Params](c *Client, notification string, params P, change
sessions = slices.Clone(c.sessions) sessions = slices.Clone(c.sessions)
} }
c.mu.Unlock() c.mu.Unlock()
notifySessions(sessions, notification, params) notifySessions(sessions, notification, params, c.logger)
} }
func (c *Client) listRoots(_ context.Context, req *ListRootsRequest) (*ListRootsResult, error) { func (c *Client) listRoots(_ context.Context, req *ListRootsRequest) (*ListRootsResult, error) {

View file

@ -508,7 +508,7 @@ func (s *Server) changeAndNotify(notification string, params Params, change func
sessions = slices.Clone(s.sessions) sessions = slices.Clone(s.sessions)
} }
s.mu.Unlock() 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. // 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] subscribedSessions := s.resourceSubscriptions[params.URI]
sessions := slices.Collect(maps.Keys(subscribedSessions)) sessions := slices.Collect(maps.Keys(subscribedSessions))
s.mu.Unlock() 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)) s.opts.Logger.Info("resource updated notification sent", "uri", params.URI, "subscriber_count", len(sessions))
return nil return nil
} }

View file

@ -15,7 +15,7 @@ import (
"context" "context"
"encoding/json" "encoding/json"
"fmt" "fmt"
"log" "log/slog"
"net/http" "net/http"
"reflect" "reflect"
"slices" "slices"
@ -381,7 +381,8 @@ const (
// notifySessions calls Notify on all the sessions. // notifySessions calls Notify on all the sessions.
// Should be called on a copy of the peer 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 { if sessions == nil {
return return
} }
@ -394,8 +395,7 @@ func notifySessions[S Session, P Params](sessions []S, method string, params P)
for _, s := range sessions { for _, s := range sessions {
req := newRequest(s, params) req := newRequest(s, params)
if err := handleNotify(ctx, method, req); err != nil { if err := handleNotify(ctx, method, req); err != nil {
// TODO(#218): surface this error better logger.Warn(fmt.Sprintf("calling %s: %v", method, err))
log.Printf("calling %s: %v", method, err)
} }
} }
} }

View file

@ -1453,7 +1453,7 @@ func (t *StreamableClientTransport) Connect(ctx context.Context) (Connection, er
done: make(chan struct{}), done: make(chan struct{}),
maxRetries: maxRetries, maxRetries: maxRetries,
strict: t.strict, strict: t.strict,
logger: t.logger, logger: ensureLogger(t.logger), // must be non-nil for safe logging
ctx: connCtx, ctx: connCtx,
cancel: cancel, cancel: cancel,
failed: make(chan struct{}), failed: make(chan struct{}),
@ -1555,9 +1555,7 @@ func (c *streamableClientConn) connectStandaloneSSE() {
// stream. // stream.
// //
// Treat this like MethodNotAllowed in non-strict mode. // 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() resp.Body.Close()
return 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. // Some servers return 200, even with an empty json body.
// //
// In strict mode, return an error to the caller. // In strict mode, return an error to the caller.
if c.logger != nil { c.logger.Warn(errMsg)
c.logger.Warn(errMsg)
}
if c.strict { if c.strict {
return errors.New(errMsg) return errors.New(errMsg)
} }