Skip to content

LoggingHandler: surface errors and use golang.org/x/time/rate for rate limiting #924

@wucm667

Description

@wucm667

Is your feature request related to a problem? Please describe.

The LoggingHandler in mcp/logging.go has two issues:

  1. Errors are silently swallowed: The Handle() method returns an error, but the comment acknowledges it will "probably be ignored":

    func (h *LoggingHandler) Handle(ctx context.Context, r slog.Record) error {
        err := h.handle(ctx, r)
        // TODO(jba): find a way to surface the error.
        // The return value will probably be ignored.
        return err
    }

    When logging fails (e.g., connection closed, server not supporting logging), the caller has no way to detect or handle the failure.

  2. Rate limiting is imprecise: The current implementation uses a simple time-interval check:

    // TODO(jba): use golang.org/x/time/rate.
    skip := time.Since(h.lastMessageSent) < h.opts.MinInterval

    This is a basic "at most one per interval" approach. It doesn't support burst tolerance, and messages during the cooldown window are silently dropped without any counter or notification.

Describe the solution you'd like

  1. Error surfacing: Provide a mechanism for callers to know when log messages fail to send. Options:

    • Add an OnError callback to LoggingHandler options
    • Return errors through the slog.Handler chain in a discoverable way
    • At minimum, log to stderr as a fallback when the MCP logging channel fails
  2. Proper rate limiting: Replace the interval check with golang.org/x/time/rate.Limiter, which supports:

    • Burst tolerance (allow N messages in quick succession)
    • Configurable rate (messages per second)
    • Standard, well-tested implementation

Describe alternatives you've considered

  • Document the limitation and let users implement their own rate limiting — but the SDK should handle this since logging is a core MCP feature.
  • Use the existing MinInterval with a burst counter — but this is reinventing what rate.Limiter already provides.

Additional context

  • File: mcp/logging.go, lines 153 and 160
  • The SDK already imports golang.org/x/sync (via singleflight in other packages), so adding golang.org/x/time/rate as a dependency is a natural extension.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions