Skip to content

Commit

Permalink
auth/clientcredentials: do not log cancelled at error level (#76)
Browse files Browse the repository at this point in the history
## Test plan

CI
  • Loading branch information
bobheadxi authored Nov 27, 2024
1 parent 7742c23 commit 8e3c388
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 15 deletions.
35 changes: 23 additions & 12 deletions auth/clientcredentials/connectrpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
13 changes: 10 additions & 3 deletions auth/clientcredentials/http.go
Original file line number Diff line number Diff line change
@@ -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"
)

Expand Down Expand Up @@ -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
}

Expand Down

0 comments on commit 8e3c388

Please sign in to comment.