Skip to content

Commit bdf88a6

Browse files
Copilotlpcox
andcommitted
Address code review feedback - Improve test quality and add TODO comments
- Replace assert.AnError with realistic fmt.Errorf() in tests - Add TODO comment acknowledging string-based error matching limitations - Suggest future improvement to use type assertions for error classification Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
1 parent f168edb commit bdf88a6

2 files changed

Lines changed: 4 additions & 2 deletions

File tree

internal/mcp/connection.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ import (
2424
var logConn = logger.New("mcp:connection")
2525

2626
// isHTTPConnectionError checks if an error is a network connection error
27-
// This helper reduces code duplication for checking common connection error patterns
27+
// This helper reduces code duplication for checking common connection error patterns.
28+
// Note: Uses string matching which is fragile but consistent with existing patterns in the codebase.
29+
// TODO: Consider using errors.Is() or type assertions (*net.OpError) for more robust error classification.
2830
func isHTTPConnectionError(err error) bool {
2931
if err == nil {
3032
return false

internal/server/call_tool_result_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ func TestNewErrorCallToolResult(t *testing.T) {
162162
}{
163163
{
164164
name: "simple error",
165-
err: assert.AnError,
165+
err: fmt.Errorf("test error"),
166166
expectError: true,
167167
},
168168
{

0 commit comments

Comments
 (0)