Merge pull request Wei-Shaw#680 from alfadb/fix/ops-normalize-nil-error-type

fix(ops): validate error_type against known whitelist before classification

Author: Wei-Shaw

backend/internal/handler/ops_error_logger.go

@@ -662,8 +662,10 @@ func OpsErrorLoggerMiddleware(ops *service.OpsService) gin.HandlerFunc {
 			requestID = c.Writer.Header().Get("x-request-id")
 		}
 
-		phase := classifyOpsPhase(parsed.ErrorType, parsed.Message, parsed.Code)
-		isBusinessLimited := classifyOpsIsBusinessLimited(parsed.ErrorType, phase, parsed.Code, status, parsed.Message)
+		normalizedType := normalizeOpsErrorType(parsed.ErrorType, parsed.Code)
+
+		phase := classifyOpsPhase(normalizedType, parsed.Message, parsed.Code)
+		isBusinessLimited := classifyOpsIsBusinessLimited(normalizedType, phase, parsed.Code, status, parsed.Message)
 
 		errorOwner := classifyOpsErrorOwner(phase, parsed.Message)
 		errorSource := classifyOpsErrorSource(phase, parsed.Message)
@@ -685,8 +687,8 @@ func OpsErrorLoggerMiddleware(ops *service.OpsService) gin.HandlerFunc {
 			UserAgent: c.GetHeader("User-Agent"),
 
 			ErrorPhase:        phase,
-			ErrorType:         normalizeOpsErrorType(parsed.ErrorType, parsed.Code),
-			Severity:          classifyOpsSeverity(parsed.ErrorType, status),
+			ErrorType:         normalizedType,
+			Severity:          classifyOpsSeverity(normalizedType, status),
 			StatusCode:        status,
 			IsBusinessLimited: isBusinessLimited,
 			IsCountTokens:     isCountTokensRequest(c),
@@ -698,7 +700,7 @@ func OpsErrorLoggerMiddleware(ops *service.OpsService) gin.HandlerFunc {
 			ErrorSource: errorSource,
 			ErrorOwner:  errorOwner,
 
-			IsRetryable: classifyOpsIsRetryable(parsed.ErrorType, status),
+			IsRetryable: classifyOpsIsRetryable(normalizedType, status),
 			RetryCount:  0,
 			CreatedAt:   time.Now(),
 		}
@@ -939,8 +941,29 @@ func guessPlatformFromPath(path string) string {
 	}
 }
 
+// isKnownOpsErrorType returns true if t is a recognized error type used by the
+// ops classification pipeline.  Upstream proxies sometimes return garbage values
+// (e.g. the Go-serialized literal "<nil>") which would pollute phase/severity
+// classification if accepted blindly.
+func isKnownOpsErrorType(t string) bool {
+	switch t {
+	case "invalid_request_error",
+		"authentication_error",
+		"rate_limit_error",
+		"billing_error",
+		"subscription_error",
+		"upstream_error",
+		"overloaded_error",
+		"api_error",
+		"not_found_error",
+		"forbidden_error":
+		return true
+	}
+	return false
+}
+
 func normalizeOpsErrorType(errType string, code string) string {
-	if errType != "" {
+	if errType != "" && isKnownOpsErrorType(errType) {
 		return errType
 	}
 	switch strings.TrimSpace(code) {

backend/internal/handler/ops_error_logger_test.go

@@ -214,3 +214,63 @@ func TestOpsErrorLoggerMiddleware_DoesNotBreakOuterMiddlewares(t *testing.T) {
 	})
 	require.Equal(t, http.StatusNoContent, rec.Code)
 }
+
+func TestIsKnownOpsErrorType(t *testing.T) {
+	known := []string{
+		"invalid_request_error",
+		"authentication_error",
+		"rate_limit_error",
+		"billing_error",
+		"subscription_error",
+		"upstream_error",
+		"overloaded_error",
+		"api_error",
+		"not_found_error",
+		"forbidden_error",
+	}
+	for _, k := range known {
+		require.True(t, isKnownOpsErrorType(k), "expected known: %s", k)
+	}
+
+	unknown := []string{"<nil>", "null", "", "random_error", "some_new_type", "<nil>\u003e"}
+	for _, u := range unknown {
+		require.False(t, isKnownOpsErrorType(u), "expected unknown: %q", u)
+	}
+}
+
+func TestNormalizeOpsErrorType(t *testing.T) {
+	tests := []struct {
+		name    string
+		errType string
+		code    string
+		want    string
+	}{
+		// Known types pass through.
+		{"known invalid_request_error", "invalid_request_error", "", "invalid_request_error"},
+		{"known rate_limit_error", "rate_limit_error", "", "rate_limit_error"},
+		{"known upstream_error", "upstream_error", "", "upstream_error"},
+
+		// Unknown/garbage types are rejected and fall through to code-based or default.
+		{"nil literal from upstream", "<nil>", "", "api_error"},
+		{"null string", "null", "", "api_error"},
+		{"random string", "something_weird", "", "api_error"},
+
+		// Unknown type but known code still maps correctly.
+		{"nil with INSUFFICIENT_BALANCE code", "<nil>", "INSUFFICIENT_BALANCE", "billing_error"},
+		{"nil with USAGE_LIMIT_EXCEEDED code", "<nil>", "USAGE_LIMIT_EXCEEDED", "subscription_error"},
+
+		// Empty type falls through to code-based mapping.
+		{"empty type with balance code", "", "INSUFFICIENT_BALANCE", "billing_error"},
+		{"empty type with subscription code", "", "SUBSCRIPTION_NOT_FOUND", "subscription_error"},
+		{"empty type no code", "", "", "api_error"},
+
+		// Known type overrides conflicting code-based mapping.
+		{"known type overrides conflicting code", "rate_limit_error", "INSUFFICIENT_BALANCE", "rate_limit_error"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := normalizeOpsErrorType(tt.errType, tt.code)
+			require.Equal(t, tt.want, got)
+		})
+	}
+}