diff --git a/app/controlplane/internal/service/service.go b/app/controlplane/internal/service/service.go index 5f1ffd1ad..40e613b5e 100644 --- a/app/controlplane/internal/service/service.go +++ b/app/controlplane/internal/service/service.go @@ -422,6 +422,30 @@ func handleUseCaseErr(err error, l *log.Helper) error { case biz.IsErrReleasedVersionImmutable(err): return status.Error(codes.FailedPrecondition, err.Error()) default: + // Client errors already converted by this function can be processed again + // (e.g. AttestationService.Store wraps storeAttestation, which converts internally). + // Propagate them instead of masking and reporting them to Sentry. + // We extract the status via GRPCStatus() instead of status.FromError because the latter + // rewrites the message of wrapped errors with the "rpc error: ..." prefix. + var gs interface{ GRPCStatus() *status.Status } + if errors.As(err, &gs) { + if s := gs.GRPCStatus(); isClientErrorCode(s.Code()) { + return s.Err() + } + } + return servicelogger.LogAndMaskErr(err, l) } } + +// isClientErrorCode returns true for the gRPC client-error codes that handleUseCaseErr +// produces, making its conversion idempotent: server-side codes keep being masked +func isClientErrorCode(c codes.Code) bool { + switch c { + case codes.Canceled, codes.InvalidArgument, codes.NotFound, codes.PermissionDenied, + codes.Unimplemented, codes.AlreadyExists, codes.FailedPrecondition: + return true + default: + return false + } +} diff --git a/app/controlplane/internal/service/service_test.go b/app/controlplane/internal/service/service_test.go index 9be990b20..939405671 100644 --- a/app/controlplane/internal/service/service_test.go +++ b/app/controlplane/internal/service/service_test.go @@ -1,5 +1,5 @@ // -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -17,13 +17,98 @@ package service import ( "context" + "errors" + "fmt" "testing" "github.com/chainloop-dev/chainloop/app/controlplane/internal/usercontext/entities" + "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz" + kerrors "github.com/go-kratos/kratos/v2/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) +func TestHandleUseCaseErr(t *testing.T) { + t.Parallel() + + immutableErr := status.Error(codes.FailedPrecondition, `version "v1.83.2+next" is released and immutable: attestations cannot be added`) + + testCases := []struct { + name string + err error + wantCode codes.Code + wantMessage string + }{ + { + name: "failed precondition status error is propagated", + err: immutableErr, + wantCode: codes.FailedPrecondition, + wantMessage: `version "v1.83.2+next" is released and immutable: attestations cannot be added`, + }, + { + name: "wrapped failed precondition keeps code and original message", + err: fmt.Errorf("saving attestation digest: %w", immutableErr), + wantCode: codes.FailedPrecondition, + wantMessage: `version "v1.83.2+next" is released and immutable: attestations cannot be added`, + }, + { + name: "released version immutable biz error maps to failed precondition", + err: fmt.Errorf("saving attestation digest: %w", biz.NewErrReleasedVersionImmutable("v1.83.2+next")), + wantCode: codes.FailedPrecondition, + wantMessage: `saving attestation digest: version "v1.83.2+next" is released and immutable: attestations cannot be added`, + }, + { + name: "already converted error is propagated unchanged when processed again", + err: handleUseCaseErr(fmt.Errorf("saving attestation digest: %w", biz.NewErrReleasedVersionImmutable("v1.83.2+next")), nil), + wantCode: codes.FailedPrecondition, + wantMessage: `saving attestation digest: version "v1.83.2+next" is released and immutable: attestations cannot be added`, + }, + { + name: "already converted not found error is propagated unchanged when processed again", + err: handleUseCaseErr(biz.NewErrNotFound("workflow"), nil), + wantCode: codes.NotFound, + wantMessage: "workflow not found", + }, + { + name: "already converted already exists error is propagated unchanged when processed again", + err: handleUseCaseErr(biz.NewErrAlreadyExists(errors.New("name taken")), nil), + wantCode: codes.AlreadyExists, + wantMessage: "duplicated: name taken", + }, + { + name: "server-side status error is still masked", + err: status.Error(codes.Unavailable, "connection to database lost"), + wantCode: codes.Internal, + wantMessage: "server error", + }, + { + name: "validation error maps to bad request", + err: biz.NewErrValidationStr("invalid input"), + wantCode: codes.InvalidArgument, + wantMessage: "validation error: invalid input", + }, + { + name: "unknown error is masked as internal server error", + err: errors.New("sensitive details"), + wantCode: codes.Internal, + wantMessage: "server error", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got := handleUseCaseErr(tc.err, nil) + require.Error(t, got) + assert.Equal(t, tc.wantCode, status.Code(got)) + assert.Equal(t, tc.wantMessage, kerrors.FromError(got).GetMessage()) + }) + } +} + func TestRequireCurrentUser(t *testing.T) { t.Parallel()