Skip to content

Consolidate proxy rejection handling in handler.go for consistent logging/tracing/error responses - #9251

Merged
lpcox merged 5 commits into
mainfrom
copilot/duplicate-code-inline-error-handling
Jul 13, 2026
Merged

Consolidate proxy rejection handling in handler.go for consistent logging/tracing/error responses#9251
lpcox merged 5 commits into
mainfrom
copilot/duplicate-code-inline-error-handling

Conversation

Copilot AI commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

internal/proxy/handler.go had repeated inline rejection logic (log + span error + JSON error response) across multiple branches, with inconsistent observability behavior between paths. This change centralizes that pattern so proxy failures are handled uniformly and easier to maintain.

  • Centralized rejection path

    • Added rejectProxyRequest(...) to encapsulate:
      • proxy error logging (logger.LogError)
      • span error recording (tracing.RecordSpanError, when span is present)
      • standardized JSON error response (httputil.WriteErrorResponse)
  • Replaced duplicated inline error blocks

    • Migrated affected branches to the helper in:
      • GraphQL body read failure
      • guard-not-initialized rejection
      • DIFC pre-phase labeling failure
      • filtered response serialization failure
      • upstream request failure
      • upstream response body read failure
  • Focused regression coverage

    • Added TestRejectProxyRequest to validate helper output shape and nil-safe behavior.
func rejectProxyRequest(w http.ResponseWriter, span oteltrace.Span, status int, code, msg string, err error) {
	logger.LogError("proxy", "Request rejected: status=%d code=%s message=%s err=%v", status, code, msg, err)
	if err == nil {
		err = errors.New(msg)
	}
	if span != nil {
		tracing.RecordSpanError(span, err, msg)
	}
	httputil.WriteErrorResponse(w, status, code, msg)
}

Copilot AI changed the title [WIP] Refactor duplicate error handling in proxy/handler.go Consolidate proxy rejection handling in handler.go for consistent logging/tracing/error responses Jul 13, 2026
Copilot finished work on behalf of lpcox July 13, 2026 13:54
Copilot AI requested a review from lpcox July 13, 2026 13:54
@lpcox
lpcox marked this pull request as ready for review July 13, 2026 14:01
Copilot AI review requested due to automatic review settings July 13, 2026 14:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Centralizes proxy rejection handling to standardize error logging, tracing, and JSON responses.

Changes:

  • Adds rejectProxyRequest.
  • Migrates six proxy failure paths.
  • Adds focused helper regression coverage.
Show a summary per file
File Description
internal/proxy/handler.go Consolidates proxy rejection handling.
internal/proxy/handler_test.go Tests nil-safe standardized responses.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 4
  • Review effort level: Medium

Comment thread internal/proxy/handler.go Outdated
Comment on lines +38 to +41
logger.LogError("proxy", "Request rejected: status=%d code=%s message=%s err=%v", status, code, msg, err)
if err == nil {
err = errors.New(msg)
}
Comment thread internal/proxy/handler.go Outdated
if err != nil {
logHandler.Printf("forwardAndReadBody: upstream request failed: method=%s path=%s err=%v", method, path, err)
httputil.WriteErrorResponse(w, http.StatusBadGateway, "bad_gateway", "upstream request failed")
rejectProxyRequest(w, span, http.StatusBadGateway, "bad_gateway", "upstream request failed", err)
Comment thread internal/proxy/handler.go Outdated
if err != nil {
logHandler.Printf("forwardAndReadBody: body read failed: method=%s path=%s status=%d err=%v", method, path, resp.StatusCode, err)
httputil.WriteErrorResponse(w, http.StatusBadGateway, "bad_gateway", "failed to read upstream response")
rejectProxyRequest(w, span, http.StatusBadGateway, "bad_gateway", "failed to read upstream response", err)
Comment thread internal/proxy/handler.go Outdated
if err != nil {
logHandler.Printf("forwardAndReadBody: upstream request failed: method=%s path=%s err=%v", method, path, err)
httputil.WriteErrorResponse(w, http.StatusBadGateway, "bad_gateway", "upstream request failed")
rejectProxyRequest(w, span, http.StatusBadGateway, "bad_gateway", "upstream request failed", err)
@lpcox

lpcox commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

@copilot address review feedback

…, fix double span recording, add request context to error wrapping

Copilot AI commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

@copilot address review feedback

Done in commit addressing all four review points:

  • Normalize err before logging: moved the if err == nil { err = errors.New(msg) } block above the logger.LogError call so both log and span see the same synthesized error.
  • Double span recording on fwdSpan: the resp == nil branch in handleWithDIFC now calls tracing.RecordSpanError(difcSpan, …) only, since rejectProxyRequest inside forwardAndReadBody already recorded on fwdSpan.
  • Upstream request error context: wraps the error as fmt.Errorf("%s %s: %w", method, path, err) before passing to the helper.
  • Body-read error context: wraps as fmt.Errorf("%s %s status=%d: %w", method, path, resp.StatusCode, err) to preserve endpoint and upstream status.

Copilot finished work on behalf of lpcox July 13, 2026 14:45
@lpcox
lpcox merged commit 4cf095f into main Jul 13, 2026
26 checks passed
@lpcox
lpcox deleted the copilot/duplicate-code-inline-error-handling branch July 13, 2026 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[duplicate-code] 🔍 Duplicate Code Pattern: Inline Error Handling in proxy/handler.go vs consolidated rejectRequest

3 participants