From 8e3c3882d12d3a3a567f078cd779cdd40c9b04b8 Mon Sep 17 00:00:00 2001
From: Robert Lin <robertlin1@gmail.com>
Date: Wed, 27 Nov 2024 15:55:07 -0800
Subject: [PATCH] auth/clientcredentials: do not log cancelled at error level
 (#76)

## Test plan

CI
---
 auth/clientcredentials/connectrpc.go | 35 ++++++++++++++++++----------
 auth/clientcredentials/http.go       | 13 ++++++++---
 2 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/auth/clientcredentials/connectrpc.go b/auth/clientcredentials/connectrpc.go
index 5a63223..7e39b2f 100644
--- a/auth/clientcredentials/connectrpc.go
+++ b/auth/clientcredentials/connectrpc.go
@@ -184,18 +184,29 @@ func extractSchemaRequiredScopes(spec connect.Spec, extension *protoimpl.Extensi
 // connectInternalError logs an error, adds it to the trace, and returns a connect
 // error with a safe message.
 func connectInternalError(ctx context.Context, logger log.Logger, err error, safeMsg string) error {
-	trace.SpanFromContext(ctx).
-		SetAttributes(
-			attribute.String("full_error", err.Error()),
-		)
-	logger.WithTrace(log.TraceContext{
+	span := trace.SpanFromContext(ctx)
+	span.SetAttributes(
+		attribute.String("safe_msg", safeMsg),
+		attribute.String("full_error", err.Error()))
+	span.SetStatus(otelcodes.Error, err.Error())
+
+	logger = logger.WithTrace(log.TraceContext{
 		TraceID: trace.SpanContextFromContext(ctx).TraceID().String(),
 		SpanID:  trace.SpanContextFromContext(ctx).SpanID().String(),
-	}).
-		AddCallerSkip(1).
-		Error(safeMsg,
-			log.String("code", connect.CodeInternal.String()),
-			log.Error(err),
-		)
-	return connect.NewError(connect.CodeInternal, errors.New(safeMsg))
+	}).AddCallerSkip(1)
+
+	// Log at different levels and return different codes depending on the type
+	// of this unexpected error.
+	if errors.Is(err, context.Canceled) {
+		code := connect.CodeCanceled
+		logger.Warn(safeMsg,
+			log.String("code", code.String()),
+			log.Error(err))
+		return connect.NewError(code, errors.New(safeMsg))
+	}
+	code := connect.CodeInternal
+	logger.Error(safeMsg,
+		log.String("code", code.String()),
+		log.Error(err))
+	return connect.NewError(code, errors.New(safeMsg))
 }
diff --git a/auth/clientcredentials/http.go b/auth/clientcredentials/http.go
index d278cc5..d88ebf9 100644
--- a/auth/clientcredentials/http.go
+++ b/auth/clientcredentials/http.go
@@ -1,10 +1,12 @@
 package clientcredentials
 
 import (
+	"context"
 	"net/http"
 
 	"github.com/sourcegraph/log"
 	"github.com/sourcegraph/sourcegraph-accounts-sdk-go/scopes"
+	"github.com/sourcegraph/sourcegraph/lib/errors"
 	"go.opentelemetry.io/otel/trace"
 )
 
@@ -47,9 +49,14 @@ func (a *HTTPAuthenticator) RequireScopes(requiredScopes scopes.Scopes, next htt
 
 		result, err := a.introspector.IntrospectToken(ctx, token)
 		if err != nil || result == nil {
-			logger.Error("error introspecting token", log.Error(err))
-			const ise = http.StatusInternalServerError
-			http.Error(w, http.StatusText(ise), ise)
+			code := http.StatusInternalServerError
+			if errors.Is(err, context.Canceled) {
+				code = http.StatusBadRequest
+				logger.Warn("error introspecting token", log.Error(err))
+			} else {
+				logger.Error("error introspecting token", log.Error(err))
+			}
+			http.Error(w, http.StatusText(code), code)
 			return
 		}