Skip to content

Commit d31c346

Browse files
committed
daemon improvements
* Supply logging adaptor to MCP library to handle logging errors from servers * Prevent hanging during pinging all servers when trying to quit (CTRL+C)
1 parent 682dbbd commit d31c346

File tree

2 files changed

+407
-154
lines changed

2 files changed

+407
-154
lines changed

internal/daemon/daemon.go

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ import (
1414

1515
"github.com/hashicorp/go-hclog"
1616
"github.com/mark3labs/mcp-go/client"
17+
"github.com/mark3labs/mcp-go/client/transport"
1718
"github.com/mark3labs/mcp-go/mcp"
19+
"github.com/mark3labs/mcp-go/util"
1820
"golang.org/x/sync/errgroup"
1921

2022
"github.com/mozilla-ai/mcpd/v2/internal/cmd"
@@ -24,6 +26,8 @@ import (
2426
"github.com/mozilla-ai/mcpd/v2/internal/runtime"
2527
)
2628

29+
var _ util.Logger = (*mcpLoggerAdapter)(nil)
30+
2731
// Daemon manages MCP server lifecycles, client connections, and health monitoring.
2832
// It should only be created using NewDaemon to ensure proper initialization.
2933
type Daemon struct {
@@ -47,6 +51,11 @@ type Daemon struct {
4751
clientHealthCheckInterval time.Duration
4852
}
4953

54+
// mcpLoggerAdapter adapts hclog.Logger to mcp-go's util.Logger interface.
55+
type mcpLoggerAdapter struct {
56+
logger hclog.Logger
57+
}
58+
5059
// NewDaemon creates a new Daemon instance with proper initialization.
5160
// Use this function instead of directly creating a Daemon struct.
5261
func NewDaemon(deps Dependencies, opt ...Option) (*Daemon, error) {
@@ -108,6 +117,12 @@ func NewDaemon(deps Dependencies, opt ...Option) (*Daemon, error) {
108117
}, nil
109118
}
110119

120+
func newMCPLoggerAdapter(logger hclog.Logger) *mcpLoggerAdapter {
121+
return &mcpLoggerAdapter{
122+
logger: logger,
123+
}
124+
}
125+
111126
// StartAndManage is a long-running method that starts configured MCP servers, and the API.
112127
// It launches regular health checks on the MCP servers, with statuses visible via API routes.
113128
func (d *Daemon) StartAndManage(ctx context.Context) error {
@@ -222,7 +237,13 @@ func (d *Daemon) startMCPServer(ctx context.Context, server runtime.Server) erro
222237

223238
logger.Debug("attempting to start server", "binary", runtimeBinary)
224239

225-
stdioClient, err := client.NewStdioMCPClient(runtimeBinary, environ, args...)
240+
mcpLogger := newMCPLoggerAdapter(logger.Named("transport"))
241+
stdioClient, err := client.NewStdioMCPClientWithOptions(
242+
runtimeBinary,
243+
environ,
244+
args,
245+
transport.WithCommandLogger(mcpLogger),
246+
)
226247
if err != nil {
227248
return fmt.Errorf("error starting MCP server: '%s': %w", server.Name(), err)
228249
}
@@ -396,12 +417,37 @@ func (d *Daemon) pingAllServers(ctx context.Context, maxTimeout time.Duration) e
396417
})
397418
}
398419

399-
_ = g.Wait()
400-
if len(errs) > 0 {
401-
return errors.Join(errs...)
420+
// Wait for all pings to complete, but allow interruption if parent context is cancelled.
421+
// This prevents the daemon from hanging during shutdown if a ping is stuck in uninterruptible I/O.
422+
done := make(chan struct{})
423+
go func() {
424+
_ = g.Wait()
425+
close(done)
426+
}()
427+
428+
select {
429+
case <-done:
430+
// All pings completed normally.
431+
if len(errs) > 0 {
432+
return errors.Join(errs...)
433+
}
434+
return nil
435+
case <-ctx.Done():
436+
// Parent context cancelled (shutdown), return immediately without waiting for pings to complete.
437+
// Any stuck pings will eventually time out or be cleaned up when the process exits.
438+
d.logger.Warn("Ping operation interrupted due to context cancellation, some pings may not have completed")
439+
return ctx.Err()
402440
}
441+
}
403442

404-
return nil
443+
// Infof implements mcp-go's Logger interface.
444+
func (a *mcpLoggerAdapter) Infof(format string, v ...any) {
445+
a.logger.Info(fmt.Sprintf(format, v...))
446+
}
447+
448+
// Errorf implements mcp-go's Logger interface.
449+
func (a *mcpLoggerAdapter) Errorf(format string, v ...any) {
450+
a.logger.Error(fmt.Sprintf(format, v...))
405451
}
406452

407453
// IsValidAddr returns an error if the address is not a valid "host:port" string.

0 commit comments

Comments
 (0)