[duplicate-code] Duplicate Code Pattern: Inconsistent HTTP Error Response Format in proxy/handler.go

[github-actions]

🔍 Duplicate Code Pattern: Inconsistent HTTP Error Response Format in proxy/handler.go

Part of duplicate code analysis: #5800

Summary

internal/proxy/handler.go uses http.Error (plain-text responses) in 7 places while the same file and package already provides httputil.WriteErrorResponse and httputil.WriteJSONResponse for consistent JSON-shaped error responses. This means some error responses to MCP proxy clients are plain text (text/plain; charset=utf-8) while others are structured JSON, breaking client-side error handling consistency.

Duplication Details

Pattern: Mixed http.Error and httputil.WriteErrorResponse/httputil.WriteJSONResponse

• Severity: Medium

• Occurrences: 7 instances of http.Error (plain text) vs 5+ uses of the JSON helpers in the same file

• Locations:

  • internal/proxy/handler.go line 83 — http.Error(w, "failed to read request body", http.StatusBadRequest)
  • internal/proxy/handler.go line 119 — http.Error(w, "access denied: unrecognized endpoint", http.StatusForbidden)
  • internal/proxy/handler.go line 155 — http.Error(w, "proxy enforcement not configured", http.StatusServiceUnavailable)
  • internal/proxy/handler.go line 169 — http.Error(w, "resource labeling failed", http.StatusBadGateway)
  • internal/proxy/handler.go line 335 — http.Error(w, "failed to serialize filtered response", http.StatusInternalServerError)
  • internal/proxy/handler.go line 404 — http.Error(w, "upstream request failed", http.StatusBadGateway)
  • internal/proxy/handler.go line 410 — http.Error(w, "failed to read upstream response", http.StatusBadGateway)

• Existing JSON helper usages (same file):

  • Line 32: httputil.WriteErrorResponse(w, http.StatusForbidden, "difc_forbidden", message)
  • Line 59: httputil.WriteJSONResponse(w, http.StatusOK, ...)
  • Line 91: httputil.WriteJSONResponse(w, http.StatusForbidden, ...)
  • Line 339: httputil.WriteJSONResponse(w, resp.StatusCode, ...)
  • Line 392: httputil.WriteJSONResponse(w, resp.StatusCode, ...)

• Code Sample (current — plain text):

  // proxy/handler.go line 83
  http.Error(w, "failed to read request body", http.StatusBadRequest)
  
  // proxy/handler.go line 404
  http.Error(w, "upstream request failed", http.StatusBadGateway)

• Code Sample (correct — JSON):

  // from line 32 — already correct pattern
  httputil.WriteErrorResponse(w, http.StatusForbidden, "difc_forbidden", message)

Impact Analysis

• Maintainability: Clients cannot reliably parse error responses; some get {"error":"...","message":"..."} JSON, others get text/plain. Any client-side error handler must branch on Content-Type.
• Bug Risk: Medium — existing httputil.WriteErrorResponse is already the stated standard (see package comment in httputil/httputil.go: "Both the server and proxy packages should use this helper so that API consumers always receive the same error shape"). The http.Error calls silently contradict this documented contract.
• Code Bloat: Minimal — each replacement is a one-line mechanical change.

Refactoring Recommendations

1. Replace all http.Error calls with httputil.WriteErrorResponse

   • Location: internal/proxy/handler.go lines 83, 119, 155, 169, 335, 404, 410
   • Choose a suitable code string for each (e.g., "bad_request", "forbidden", "service_unavailable", "bad_gateway", "internal_error")
   • Example replacement:

     // Before
     http.Error(w, "upstream request failed", http.StatusBadGateway)
     // After
     httputil.WriteErrorResponse(w, http.StatusBadGateway, "bad_gateway", "upstream request failed")

   • Estimated effort: < 1 hour
   • Benefits: uniform {"error":"...","message":"..."} JSON shape across all proxy error paths

2. Add a lint rule or test to prevent future regression

   • Consider a grep-based test in the integration suite that asserts http.Error is not called in proxy/handler.go

Implementation Checklist

• Review each http.Error call and pick an appropriate code string
• Replace all 7 calls with httputil.WriteErrorResponse
• Run make test-all to verify no tests break
• Optionally add integration test assertion to prevent regression

Parent Issue

See parent analysis report: #5800
Related to #5800

Generated by Duplicate Code Detector · ● 4.1M · ◷

• expires on May 23, 2026, 3:42 AM UTC