Part of duplicate code analysis: #2907

Summary

In internal/server/auth.go, and similar locations in handlers.go and routed.go, every guard/auth rejection follows the same 3-step trio: structured log → runtime error log → HTTP error response. This 3–4 line block repeats for each distinct failure case and must be kept in sync across all sites.

Duplication Details

Pattern: Log + runtime-log + writeErrorResponse trio on every rejection path

• Severity: Medium

• Occurrences: 3+ distinct sites (2 in auth.go, 1+ in handlers.go/routed.go)

• Locations:

  • internal/server/auth.go lines ~48–52 (missing auth header)
  • internal/server/auth.go lines ~56–60 (invalid API key)
  • internal/server/handlers.go lines ~42–45 (method not allowed)
  • internal/server/routed.go lines ~25–28 (shutdown check)

• Code Sample (pattern from auth.go):

  // Case 1 – missing header
  if authHeader == "" {
      logger.LogErrorMd("auth", "Authentication failed: missing Authorization header, remote=%s, path=%s", r.RemoteAddr, r.URL.Path)
      logRuntimeError("authentication_failed", "missing_auth_header", r, nil)
      writeErrorResponse(w, http.StatusUnauthorized, "unauthorized", "missing Authorization header")
      return
  }
  
  // Case 2 – invalid key (nearly identical structure)
  if authHeader != apiKey {
      logger.LogErrorMd("auth", "Authentication failed: invalid API key, remote=%s, path=%s", r.RemoteAddr, r.URL.Path)
      logRuntimeError("authentication_failed", "invalid_api_key", r, nil)
      writeErrorResponse(w, http.StatusUnauthorized, "unauthorized", "invalid API key")
      return
  }

Impact Analysis

• Maintainability: Adding a new logging destination (e.g., metrics, audit log) requires touching every rejection site
• Bug Risk: One site could be silently missing a log step after a merge conflict
• Code Bloat: ~6–8 lines per case; with 4+ cases this is ~30 lines of structural duplication

Refactoring Recommendations

1. Extract a rejectRequest helper in internal/server/http_helpers.go:

   // rejectRequest logs the rejection, records a runtime error, and writes an HTTP error response.
   func rejectRequest(w http.ResponseWriter, r *http.Request, status int, code, msg, logCategory, runtimeErrType string) {
       logger.LogErrorMd(logCategory, "Request rejected: %s, remote=%s, path=%s", msg, r.RemoteAddr, r.URL.Path)
       logRuntimeError(runtimeErrType, msg, r, nil)
       writeErrorResponse(w, status, code, msg)
   }

   Usage becomes:

   if authHeader == "" {
       rejectRequest(w, r, http.StatusUnauthorized, "unauthorized", "missing Authorization header", "auth", "missing_auth_header")
       return
   }

   Estimated effort: ~1 hour. Zero behavior change; improves consistency.

2. Alternative: Keep per-site log messages but extract only logRuntimeError + writeErrorResponse into a helper, preserving site-specific log detail.

Implementation Checklist

• Add rejectRequest (or equivalent) helper to http_helpers.go
• Refactor auth.go rejection cases
• Refactor handlers.go rejection cases
• Refactor routed.go rejection cases
• Run make test to verify no regressions

Parent Issue

See parent analysis report: #2907
Related to #2907

Generated by Duplicate Code Detector · ◷

• expires on Apr 7, 2026, 6:02 AM UTC