Skip to content

Commit 1a97a0e

Browse files
Copilotlpcox
Copilot
and
lpcox
authored
refactor: extract shared ConnectionErrorContext + LogConnectionError to deduplicate connection error diagnostics
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/f1398685-5ca9-46a3-9c73-fc8c4e04814b
1 parent 18ad57a commit 1a97a0e

4 files changed

Lines changed: 540 additions & 54 deletions

File tree

‎internal/launcher/log_helpers.go‎

Lines changed: 11 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/github/gh-aw-mcpg/internal/config"
1010
"github.com/github/gh-aw-mcpg/internal/logger"
1111
"github.com/github/gh-aw-mcpg/internal/logger/sanitize"
12+
"github.com/github/gh-aw-mcpg/internal/mcp"
1213
)
1314

1415
// sessionSuffix returns a formatted session suffix for log messages
@@ -69,29 +70,16 @@ func (l *Launcher) logEnvPassthrough(args []string) {
6970

7071
// logLaunchError logs detailed launch failure diagnostics
7172
func (l *Launcher) logLaunchError(serverID, sessionID string, err error, serverCfg *config.ServerConfig, isDirectCommand bool) {
72-
logger.LogErrorWithServer(serverID, "backend", "Failed to launch MCP backend server%s: server=%s%s, error=%v",
73-
sessionSuffix(sessionID), serverID, sessionSuffix(sessionID), err)
74-
log.Printf("[LAUNCHER] ❌ FAILED to launch server '%s'%s", serverID, sessionSuffix(sessionID))
75-
log.Printf("[LAUNCHER] Error: %v", err)
76-
log.Printf("[LAUNCHER] Debug Information:")
77-
log.Printf("[LAUNCHER] - Command: %s", serverCfg.Command)
78-
log.Printf("[LAUNCHER] - Args: %v", sanitize.SanitizeArgs(serverCfg.Args))
79-
log.Printf("[LAUNCHER] - Env vars: %v", sanitize.TruncateSecretMap(serverCfg.Env))
80-
log.Printf("[LAUNCHER] - Running in container: %v", l.runningInContainer)
81-
log.Printf("[LAUNCHER] - Is direct command: %v", isDirectCommand)
82-
log.Printf("[LAUNCHER] - Startup timeout: %v", l.startupTimeout)
83-
84-
if isDirectCommand && l.runningInContainer {
85-
log.Printf("[LAUNCHER] ⚠️ Possible causes:")
86-
log.Printf("[LAUNCHER] - Command '%s' may not be installed in the gateway container", serverCfg.Command)
87-
log.Printf("[LAUNCHER] - Consider using 'container' config instead of 'command'")
88-
log.Printf("[LAUNCHER] - Or add '%s' to the gateway's Dockerfile", serverCfg.Command)
89-
} else if isDirectCommand {
90-
log.Printf("[LAUNCHER] ⚠️ Possible causes:")
91-
log.Printf("[LAUNCHER] - Command '%s' may not be in PATH", serverCfg.Command)
92-
log.Printf("[LAUNCHER] - Check if '%s' is installed: which %s", serverCfg.Command, serverCfg.Command)
93-
log.Printf("[LAUNCHER] - Verify file permissions and execute bit")
94-
}
73+
mcp.LogConnectionError(mcp.ConnectionErrorContext{
74+
ServerID: serverID,
75+
SessionID: sessionID,
76+
Command: serverCfg.Command,
77+
Args: serverCfg.Args,
78+
Env: serverCfg.Env,
79+
RunningInContainer: l.runningInContainer,
80+
IsDirectCommand: isDirectCommand,
81+
StartupTimeout: l.startupTimeout,
82+
}, err)
9583
}
9684

9785
// logTimeoutError logs startup timeout diagnostics

‎internal/mcp/connection.go‎

Lines changed: 6 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -133,38 +133,13 @@ func NewConnection(ctx context.Context, serverID, command string, args []string,
133133
cancel()
134134
stderrPipeWriter.Close() // Close pipe to stop the stderr streaming goroutine
135135

136-
// Enhanced error context for debugging
137-
logger.LogErrorMd("backend", "MCP backend connection failed, command=%s, args=%v, error=%v", command, sanitize.SanitizeArgs(expandedArgs), err)
138-
log.Printf("❌ MCP Connection Failed:")
139-
log.Printf(" Command: %s", command)
140-
log.Printf(" Args: %v", sanitize.SanitizeArgs(expandedArgs))
141-
log.Printf(" Error: %v", err)
142-
143-
// Log captured stderr output from the container/process
144136
stderrOutput := strings.TrimSpace(stderrBuf.String())
145-
if stderrOutput != "" {
146-
sanitizedStderr := sanitize.SanitizeString(stderrOutput)
147-
logger.LogErrorMd("backend", "MCP backend stderr output:\n%s", sanitizedStderr)
148-
log.Printf(" 📋 Container/Process stderr output:")
149-
for _, line := range strings.Split(sanitizedStderr, "\n") {
150-
log.Printf(" %s", line)
151-
}
152-
}
153-
154-
// Check if it's a command not found error
155-
if strings.Contains(err.Error(), "executable file not found") ||
156-
strings.Contains(err.Error(), "no such file or directory") {
157-
logger.LogErrorMd("backend", "MCP backend command not found, command=%s", command)
158-
log.Printf(" ⚠️ Command '%s' not found in PATH", command)
159-
log.Printf(" ⚠️ Verify the command is installed and executable")
160-
}
161-
162-
// Check if it's a connection/protocol error
163-
if strings.Contains(err.Error(), "EOF") || strings.Contains(err.Error(), "broken pipe") {
164-
logger.LogErrorMd("backend", "MCP backend connection/protocol error, command=%s", command)
165-
log.Printf(" ⚠️ Process started but terminated unexpectedly")
166-
log.Printf(" ⚠️ Check if the command supports MCP protocol over stdio")
167-
}
137+
LogConnectionError(ConnectionErrorContext{
138+
ServerID: serverID,
139+
Command: command,
140+
Args: expandedArgs,
141+
StderrOutput: stderrOutput,
142+
}, err)
168143

169144
logConn.Printf("Connection failed: command=%s, error=%v", command, err)
170145
return nil, fmt.Errorf("failed to connect: %w", err)

‎internal/mcp/errors.go‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
package mcp
2+
3+
import (
4+
"log"
5+
"strings"
6+
"time"
7+
8+
"github.com/github/gh-aw-mcpg/internal/logger"
9+
"github.com/github/gh-aw-mcpg/internal/logger/sanitize"
10+
)
11+
12+
// ConnectionErrorContext holds all context needed to produce a detailed connection
13+
// failure diagnostic. Fields left at their zero values are omitted from the output.
14+
type ConnectionErrorContext struct {
15+
ServerID string
16+
SessionID string
17+
Command string
18+
Args []string
19+
Env map[string]string
20+
RunningInContainer bool
21+
IsDirectCommand bool
22+
StartupTimeout time.Duration
23+
StderrOutput string
24+
}
25+
26+
// LogConnectionError logs detailed diagnostics for a connection failure, including
27+
// command context, captured stderr, and actionable hints based on the error type
28+
// and execution environment. All callers (launcher and mcp connection) use this
29+
// single function so that hint analysis and output format remain consistent.
30+
func LogConnectionError(errCtx ConnectionErrorContext, err error) {
31+
suffix := ""
32+
if errCtx.SessionID != "" {
33+
suffix = " for session '" + errCtx.SessionID + "'"
34+
}
35+
36+
// Structured log via file logger.
37+
if errCtx.ServerID != "" {
38+
logger.LogErrorWithServer(errCtx.ServerID, "backend",
39+
"MCP backend connection failed%s: server=%s, command=%s, args=%v, error=%v",
40+
suffix, errCtx.ServerID, errCtx.Command, sanitize.SanitizeArgs(errCtx.Args), err)
41+
} else {
42+
logger.LogErrorMd("backend",
43+
"MCP backend connection failed, command=%s, args=%v, error=%v",
44+
errCtx.Command, sanitize.SanitizeArgs(errCtx.Args), err)
45+
}
46+
47+
// Human-readable console output.
48+
if errCtx.ServerID != "" {
49+
log.Printf("[LAUNCHER] ❌ FAILED to connect to server '%s'%s", errCtx.ServerID, suffix)
50+
} else {
51+
log.Printf("[LAUNCHER] ❌ MCP Connection Failed")
52+
}
53+
log.Printf("[LAUNCHER] Error: %v", err)
54+
log.Printf("[LAUNCHER] Debug Information:")
55+
log.Printf("[LAUNCHER] - Command: %s", errCtx.Command)
56+
log.Printf("[LAUNCHER] - Args: %v", sanitize.SanitizeArgs(errCtx.Args))
57+
if len(errCtx.Env) > 0 {
58+
log.Printf("[LAUNCHER] - Env vars: %v", sanitize.TruncateSecretMap(errCtx.Env))
59+
}
60+
if errCtx.RunningInContainer || errCtx.IsDirectCommand {
61+
log.Printf("[LAUNCHER] - Running in container: %v", errCtx.RunningInContainer)
62+
log.Printf("[LAUNCHER] - Is direct command: %v", errCtx.IsDirectCommand)
63+
}
64+
if errCtx.StartupTimeout > 0 {
65+
log.Printf("[LAUNCHER] - Startup timeout: %v", errCtx.StartupTimeout)
66+
}
67+
68+
// Log captured stderr output from the container/process.
69+
if errCtx.StderrOutput != "" {
70+
sanitizedStderr := sanitize.SanitizeString(errCtx.StderrOutput)
71+
logger.LogErrorMd("backend", "MCP backend stderr output:\n%s", sanitizedStderr)
72+
log.Printf("[LAUNCHER] 📋 Process stderr output:")
73+
for _, line := range strings.Split(sanitizedStderr, "\n") {
74+
log.Printf("[LAUNCHER] %s", line)
75+
}
76+
}
77+
78+
// Hints based on execution context (launcher-specific).
79+
if errCtx.IsDirectCommand && errCtx.RunningInContainer {
80+
log.Printf("[LAUNCHER] ⚠️ Possible causes:")
81+
log.Printf("[LAUNCHER] - Command '%s' may not be installed in the gateway container", errCtx.Command)
82+
log.Printf("[LAUNCHER] - Consider using 'container' config instead of 'command'")
83+
log.Printf("[LAUNCHER] - Or add '%s' to the gateway's Dockerfile", errCtx.Command)
84+
} else if errCtx.IsDirectCommand {
85+
log.Printf("[LAUNCHER] ⚠️ Possible causes:")
86+
log.Printf("[LAUNCHER] - Command '%s' may not be in PATH", errCtx.Command)
87+
log.Printf("[LAUNCHER] - Check if '%s' is installed: which %s", errCtx.Command, errCtx.Command)
88+
log.Printf("[LAUNCHER] - Verify file permissions and execute bit")
89+
}
90+
91+
// Hints based on error message content.
92+
errStr := err.Error()
93+
if strings.Contains(errStr, "executable file not found") || strings.Contains(errStr, "no such file or directory") {
94+
logger.LogErrorMd("backend", "MCP backend command not found, command=%s", errCtx.Command)
95+
log.Printf("[LAUNCHER] ⚠️ Command '%s' not found in PATH", errCtx.Command)
96+
log.Printf("[LAUNCHER] ⚠️ Verify the command is installed and executable")
97+
}
98+
99+
if strings.Contains(errStr, "EOF") || strings.Contains(errStr, "broken pipe") {
100+
logger.LogErrorMd("backend", "MCP backend connection/protocol error, command=%s", errCtx.Command)
101+
log.Printf("[LAUNCHER] ⚠️ Process started but terminated unexpectedly")
102+
log.Printf("[LAUNCHER] ⚠️ Check if the command supports MCP protocol over stdio")
103+
}
104+
}

0 commit comments

Comments
 (0)