-
Notifications
You must be signed in to change notification settings - Fork 16
Open
Description
Part of duplicate code analysis: #2300
Summary
The block that forwards a request to the upstream GitHub API and reads the response body — including identical error messages — appears twice in internal/proxy/handler.go (handleWithDIFC and passthrough).
Duplication Details
Pattern: forwardToGitHub → io.ReadAll → error handling
-
Severity: Low
-
Occurrences: 2 instances in the same file
-
Locations:
internal/proxy/handler.go(lines 148–165) — insidehandleWithDIFCinternal/proxy/handler.go(lines 279–291) — insidepassthrough
-
Code Sample (both instances are structurally identical):
resp, err := s.forwardToGitHub(ctx, method, path, body, contentType, clientAuth) if err != nil { http.Error(w, "upstream request failed", http.StatusBadGateway) return } defer resp.Body.Close() respBody, err := io.ReadAll(resp.Body) if err != nil { http.Error(w, "failed to read upstream response", http.StatusBadGateway) return }
Even the error strings (
"upstream request failed","failed to read upstream response") are identical between the two sites.
Impact Analysis
- Maintainability: If error handling needs to change (e.g., logging, status code), both sites must be updated.
- Bug Risk: The error messages are already identical; a future edit to one site might silently diverge.
- Code Bloat: ~10 lines that could be 1–2 lines each via a shared helper.
Refactoring Recommendations
- Extract a
forwardAndReadBodyhelper- Add to
internal/proxy/handler.go(orhttp_helpers.gofrom Pattern 1):// forwardAndReadBody forwards the request to GitHub and returns the raw response body. // On error it writes an appropriate HTTP error to w and returns nil, nil. func (h *proxyHandler) forwardAndReadBody( w http.ResponseWriter, ctx context.Context, method, path string, body io.Reader, contentType, clientAuth string, ) (*http.Response, []byte) { resp, err := h.server.forwardToGitHub(ctx, method, path, body, contentType, clientAuth) if err != nil { http.Error(w, "upstream request failed", http.StatusBadGateway) return nil, nil } respBody, err := io.ReadAll(resp.Body) resp.Body.Close() if err != nil { http.Error(w, "failed to read upstream response", http.StatusBadGateway) return nil, nil } return resp, respBody }
- Replace both blocks with calls to
forwardAndReadBody. - Estimated effort: ~1 hour (including test validation)
- Benefits: single error-message source of truth; easier to add logging/tracing later
- Add to
Implementation Checklist
- Review both duplicated forwarding blocks in
internal/proxy/handler.go - Extract
forwardAndReadBodyhelper method - Replace both inline blocks with helper calls
- Verify tests still pass (
make test) - Confirm no functionality broken
Parent Issue
See parent analysis report: #2300
Related to #2300
Generated by Duplicate Code Detector · ◷
- expires on Mar 29, 2026, 3:03 AM UTC
Reactions are currently unavailable
Metadata
Metadata
Assignees
Type
Fields
Give feedbackNo fields configured for issues without a type.