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
-
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.
-
Alternative: Keep per-site log messages but extract only logRuntimeError + writeErrorResponse into a helper, preserving site-specific log detail.
Implementation Checklist
Parent Issue
See parent analysis report: #2907
Related to #2907
Generated by Duplicate Code Detector · ◷
Part of duplicate code analysis: #2907
Summary
In
internal/server/auth.go, and similar locations inhandlers.goandrouted.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.golines ~48–52 (missing auth header)internal/server/auth.golines ~56–60 (invalid API key)internal/server/handlers.golines ~42–45 (method not allowed)internal/server/routed.golines ~25–28 (shutdown check)Code Sample (pattern from
auth.go):Impact Analysis
Refactoring Recommendations
Extract a
rejectRequesthelper ininternal/server/http_helpers.go:Usage becomes:
Estimated effort: ~1 hour. Zero behavior change; improves consistency.
Alternative: Keep per-site log messages but extract only
logRuntimeError + writeErrorResponseinto a helper, preserving site-specific log detail.Implementation Checklist
rejectRequest(or equivalent) helper tohttp_helpers.goauth.gorejection caseshandlers.gorejection casesrouted.gorejection casesmake testto verify no regressionsParent Issue
See parent analysis report: #2907
Related to #2907