Skip to content

Resolve isHTTPConnectionError TODO and update MCPProtocolVersion to 2025-11-25 #1619

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lpcox merged 2 commits into from
Mar 6, 2026
Merged

Resolve isHTTPConnectionError TODO and update MCPProtocolVersion to 2025-11-25 #1619

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
01033d6
Initial plan
Copilot Mar 6, 2026
15bb898
Fix isHTTPConnectionError with errors.As and update MCPProtocolVersio…
Copilot Mar 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions internal/mcp/connection_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ package mcp
import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net"
"net/http"
"net/http/httptest"
"strings"
Expand Down Expand Up @@ -639,22 +641,32 @@ func TestIsHTTPConnectionError(t *testing.T) {
expected: false,
},
{
name: "connection refused error",
err: fmt.Errorf("dial tcp: connection refused"),
name: "net.OpError dial connection refused",
err: &net.OpError{Op: "dial", Net: "tcp", Err: errors.New("connection refused")},
expected: true,
},
{
name: "no such host error",
err: fmt.Errorf("dial tcp: lookup example.invalid: no such host"),
name: "net.OpError dial no such host",
err: &net.OpError{Op: "dial", Net: "tcp", Err: &net.DNSError{Err: "no such host", IsNotFound: true}},
expected: true,
},
{
name: "network is unreachable error",
err: fmt.Errorf("dial tcp: network is unreachable"),
name: "net.OpError dial network unreachable",
err: &net.OpError{Op: "dial", Net: "tcp", Err: errors.New("network is unreachable")},
expected: true,
},
{
name: "other error",
name: "net.OpError non-dial (read) is not a connection error",
err: &net.OpError{Op: "read", Net: "tcp", Err: errors.New("broken pipe")},
expected: false,
},
{
name: "wrapped net.OpError dial via fmt.Errorf",
err: fmt.Errorf("Post %q: %w", "http://example.com", &net.OpError{Op: "dial", Net: "tcp", Err: errors.New("connection refused")}),
expected: true,
},
Comment on lines +664 to +667
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The “wrapped” test case uses fmt.Errorf(...%w...), but http.Client.Do typically wraps dial failures in *url.Error. If the goal is to validate the real error chain that executeHTTPRequest will see, use a url.Error wrapper here so the test covers *url.Error -> *net.OpError unwrapping explicitly.

Copilot uses AI. Check for mistakes.
{
name: "plain string error",
err: fmt.Errorf("some other error"),
expected: false,
},
Expand Down
20 changes: 11 additions & 9 deletions internal/mcp/http_transport.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io"
"log/slog"
"net"
"net/http"
"strings"
"sync/atomic"
Expand All @@ -31,7 +33,7 @@ const (
)

// MCPProtocolVersion is the MCP protocol version used in initialization requests.
const MCPProtocolVersion = "2024-11-05"
const MCPProtocolVersion = "2025-11-25"
Comment on lines 35 to +36
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MCPProtocolVersion is used as a single source of truth, but there are still many docs/tests/examples in the repo that hard-code the old value (2024-11-05). To avoid confusing drift, update those references (or have tests/docs refer to this constant where feasible) so the documented/example protocolVersion matches what the code now sends.

This issue also appears on line 35 of the same file.

Copilot uses AI. Check for mistakes.

// requestIDCounter is used to generate unique request IDs for HTTP requests
var requestIDCounter uint64
Expand All @@ -46,18 +48,18 @@ type httpRequestResult struct {
// transportConnector is a function that creates an SDK transport for a given URL and HTTP client
type transportConnector func(url string, httpClient *http.Client) sdk.Transport

// isHTTPConnectionError checks if an error is a network connection error
// This helper reduces code duplication for checking common connection error patterns.
// Note: Uses string matching which is fragile but consistent with existing patterns in the codebase.
// TODO: Consider using errors.Is() or type assertions (*net.OpError) for more robust error classification.
// isHTTPConnectionError checks if an error is a network connection error.
// It uses errors.As to inspect the underlying *net.OpError for dial operations,
// which covers connection refused, no such host, and network unreachable errors.
func isHTTPConnectionError(err error) bool {
if err == nil {
return false
}
errMsg := err.Error()
return strings.Contains(errMsg, "connection refused") ||
strings.Contains(errMsg, "no such host") ||
strings.Contains(errMsg, "network is unreachable")
var opErr *net.OpError
if errors.As(err, &opErr) {
return opErr.Op == "dial"
}
return false
}

// parseSSEResponse extracts JSON data from SSE-formatted response
Expand Down
Loading