Skip to content

Commit a32a757

Browse files
Fix panic when fetching resources fails due to network error (#1506)
* fix panic due to defer reading body of failed request --------- Co-authored-by: Adam Holt <omgitsads@github.com>
1 parent 2b55513 commit a32a757

File tree

2 files changed

+43
-2
lines changed

2 files changed

+43
-2
lines changed

pkg/github/repository_resource.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,13 +191,14 @@ func RepositoryResourceContentsHandler(resourceURITemplate *uritemplate.Template
191191
}
192192

193193
resp, err := rawClient.GetRawContent(ctx, owner, repo, path, rawOpts)
194+
if err != nil {
195+
return nil, fmt.Errorf("failed to get raw content: %w", err)
196+
}
194197
defer func() {
195198
_ = resp.Body.Close()
196199
}()
197200
// If the raw content is not found, we will fall back to the GitHub API (in case it is a directory)
198201
switch {
199-
case err != nil:
200-
return nil, fmt.Errorf("failed to get raw content: %w", err)
201202
case resp.StatusCode == http.StatusOK:
202203
ext := filepath.Ext(path)
203204
mimeType := resp.Header.Get("Content-Type")

pkg/github/repository_resource_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package github
22

33
import (
44
"context"
5+
"errors"
56
"net/http"
67
"net/url"
78
"testing"
@@ -12,6 +13,15 @@ import (
1213
"github.com/stretchr/testify/require"
1314
)
1415

16+
// errorTransport is a http.RoundTripper that always returns an error.
17+
type errorTransport struct {
18+
err error
19+
}
20+
21+
func (t *errorTransport) RoundTrip(*http.Request) (*http.Response, error) {
22+
return nil, t.err
23+
}
24+
1525
type resourceResponseType int
1626

1727
const (
@@ -272,3 +282,33 @@ func Test_repositoryResourceContents(t *testing.T) {
272282
})
273283
}
274284
}
285+
286+
// Test_repositoryResourceContentsHandler_NetworkError tests that a network error
287+
// during raw content fetch does not cause a panic (nil response body dereference).
288+
func Test_repositoryResourceContentsHandler_NetworkError(t *testing.T) {
289+
base, _ := url.Parse("https://raw.example.com/")
290+
networkErr := errors.New("network error: connection refused")
291+
292+
httpClient := &http.Client{Transport: &errorTransport{err: networkErr}}
293+
client := github.NewClient(httpClient)
294+
mockRawClient := raw.NewClient(client, base)
295+
deps := BaseDeps{
296+
Client: client,
297+
RawClient: mockRawClient,
298+
}
299+
ctx := ContextWithDeps(context.Background(), deps)
300+
301+
handler := RepositoryResourceContentsHandler(repositoryResourceContentURITemplate)
302+
303+
request := &mcp.ReadResourceRequest{
304+
Params: &mcp.ReadResourceParams{
305+
URI: "repo://owner/repo/contents/README.md",
306+
},
307+
}
308+
309+
// This should not panic, even though the HTTP client returns an error
310+
resp, err := handler(ctx, request)
311+
require.Error(t, err)
312+
require.Nil(t, resp)
313+
require.ErrorContains(t, err, "failed to get raw content")
314+
}

0 commit comments

Comments
 (0)