Skip to content

Commit 7e0380c

Browse files
Fix response body handling and improve thread safety in tests
Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
1 parent 6e0e72a commit 7e0380c

File tree

2 files changed

+21
-9
lines changed

2 files changed

+21
-9
lines changed

pkg/github/issues.go

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -809,6 +809,7 @@ func AddSubIssue(ctx context.Context, client *github.Client, owner string, repo
809809

810810
var lastResp *github.Response
811811
var lastErr error
812+
var lastBody []byte
812813

813814
for attempt := 0; attempt < maxRetries; attempt++ {
814815
if attempt > 0 {
@@ -837,8 +838,11 @@ func AddSubIssue(ctx context.Context, client *github.Client, owner string, repo
837838
// Read the body to check for priority conflict
838839
body, readErr := io.ReadAll(resp.Body)
839840
_ = resp.Body.Close()
840-
if readErr == nil && strings.Contains(string(body), "Priority has already been taken") {
841-
shouldRetry = true
841+
if readErr == nil {
842+
lastBody = body
843+
if strings.Contains(string(body), "Priority has already been taken") {
844+
shouldRetry = true
845+
}
842846
}
843847
}
844848

@@ -859,12 +863,19 @@ func AddSubIssue(ctx context.Context, client *github.Client, owner string, repo
859863

860864
// Handle non-201 status codes after retries exhausted
861865
if lastResp != nil && lastResp.StatusCode != http.StatusCreated {
862-
defer func() { _ = lastResp.Body.Close() }()
863-
body, err := io.ReadAll(lastResp.Body)
864-
if err != nil {
865-
return nil, fmt.Errorf("failed to read response body: %w", err)
866+
// Use the body we already read during retry attempts if available
867+
if len(lastBody) > 0 {
868+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to add sub-issue", lastResp, lastBody), nil
869+
}
870+
// Otherwise try to read the body if it's still available
871+
if lastResp.Body != nil {
872+
body, err := io.ReadAll(lastResp.Body)
873+
_ = lastResp.Body.Close()
874+
if err != nil {
875+
return nil, fmt.Errorf("failed to read response body: %w", err)
876+
}
877+
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to add sub-issue", lastResp, body), nil
866878
}
867-
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to add sub-issue", lastResp, body), nil
868879
}
869880

870881
// This should not be reached in normal operation

pkg/github/issues_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3044,10 +3044,11 @@ func Test_AddSubIssue_RetryLogic(t *testing.T) {
30443044
name: "successful retry after priority conflict",
30453045
mockedClient: func() *http.Client {
30463046
// Create a handler that fails twice with priority conflict, then succeeds
3047-
callCount := 0
3047+
var callCount int32
30483048
handler := func(w http.ResponseWriter, _ *http.Request) {
3049+
count := callCount
30493050
callCount++
3050-
if callCount <= 2 {
3051+
if count < 2 {
30513052
// First two calls fail with priority conflict
30523053
w.WriteHeader(http.StatusUnprocessableEntity)
30533054
_, _ = w.Write([]byte(`{"message": "An error occurred while adding the sub-issue to the parent issue. Priority has already been taken"}`))

0 commit comments

Comments
 (0)