Skip to content

Commit 2c91c06

Browse files
committed
fix(controlplane): propagate FailedPrecondition errors instead of masking them
When an error already converted by handleUseCaseErr is processed again (e.g. AttestationService.Store wrapping storeAttestation), the resulting gRPC FailedPrecondition status error was not recognized, so it was reported to Sentry and masked as a generic internal server error, hiding the actionable message (such as pushing an attestation to a released, immutable project version) from the client. Pass through gRPC FailedPrecondition status errors in the default branch, making the conversion idempotent: the error is no longer reported to Sentry and the original message reaches the client. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev> Chainloop-Trace-Sessions: 43473f24-f00c-4ba7-8fac-82209c736b02
1 parent c6ae8cd commit 2c91c06

2 files changed

Lines changed: 81 additions & 1 deletion

File tree

app/controlplane/internal/service/service.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,19 @@ func handleUseCaseErr(err error, l *log.Helper) error {
422422
case biz.IsErrReleasedVersionImmutable(err):
423423
return status.Error(codes.FailedPrecondition, err.Error())
424424
default:
425+
// Business precondition failures (e.g. adding attestations to a released project version)
426+
// are client errors: propagate them instead of masking and reporting them to Sentry.
427+
// They can reach this point as gRPC status errors when an error already converted by
428+
// this function is processed again (e.g. Store wrapping storeAttestation).
429+
// We extract the status via GRPCStatus() instead of status.FromError because the latter
430+
// rewrites the message of wrapped errors with the "rpc error: ..." prefix.
431+
var gs interface{ GRPCStatus() *status.Status }
432+
if errors.As(err, &gs) {
433+
if s := gs.GRPCStatus(); s.Code() == codes.FailedPrecondition {
434+
return s.Err()
435+
}
436+
}
437+
425438
return servicelogger.LogAndMaskErr(err, l)
426439
}
427440
}

app/controlplane/internal/service/service_test.go

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//
2-
// Copyright 2023 The Chainloop Authors.
2+
// Copyright 2023-2026 The Chainloop Authors.
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
55
// you may not use this file except in compliance with the License.
@@ -17,13 +17,80 @@ package service
1717

1818
import (
1919
"context"
20+
"errors"
21+
"fmt"
2022
"testing"
2123

2224
"github.com/chainloop-dev/chainloop/app/controlplane/internal/usercontext/entities"
25+
"github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz"
26+
kerrors "github.com/go-kratos/kratos/v2/errors"
2327
"github.com/stretchr/testify/assert"
2428
"github.com/stretchr/testify/require"
29+
"google.golang.org/grpc/codes"
30+
"google.golang.org/grpc/status"
2531
)
2632

33+
func TestHandleUseCaseErr(t *testing.T) {
34+
t.Parallel()
35+
36+
immutableErr := status.Error(codes.FailedPrecondition, `version "v1.83.2+next" is released and immutable: attestations cannot be added`)
37+
38+
testCases := []struct {
39+
name string
40+
err error
41+
wantCode codes.Code
42+
wantMessage string
43+
}{
44+
{
45+
name: "failed precondition status error is propagated",
46+
err: immutableErr,
47+
wantCode: codes.FailedPrecondition,
48+
wantMessage: `version "v1.83.2+next" is released and immutable: attestations cannot be added`,
49+
},
50+
{
51+
name: "wrapped failed precondition keeps code and original message",
52+
err: fmt.Errorf("saving attestation digest: %w", immutableErr),
53+
wantCode: codes.FailedPrecondition,
54+
wantMessage: `version "v1.83.2+next" is released and immutable: attestations cannot be added`,
55+
},
56+
{
57+
name: "released version immutable biz error maps to failed precondition",
58+
err: fmt.Errorf("saving attestation digest: %w", biz.NewErrReleasedVersionImmutable("v1.83.2+next")),
59+
wantCode: codes.FailedPrecondition,
60+
wantMessage: `saving attestation digest: version "v1.83.2+next" is released and immutable: attestations cannot be added`,
61+
},
62+
{
63+
name: "already converted error is propagated unchanged when processed again",
64+
err: handleUseCaseErr(fmt.Errorf("saving attestation digest: %w", biz.NewErrReleasedVersionImmutable("v1.83.2+next")), nil),
65+
wantCode: codes.FailedPrecondition,
66+
wantMessage: `saving attestation digest: version "v1.83.2+next" is released and immutable: attestations cannot be added`,
67+
},
68+
{
69+
name: "validation error maps to bad request",
70+
err: biz.NewErrValidationStr("invalid input"),
71+
wantCode: codes.InvalidArgument,
72+
wantMessage: "validation error: invalid input",
73+
},
74+
{
75+
name: "unknown error is masked as internal server error",
76+
err: errors.New("sensitive details"),
77+
wantCode: codes.Internal,
78+
wantMessage: "server error",
79+
},
80+
}
81+
82+
for _, tc := range testCases {
83+
t.Run(tc.name, func(t *testing.T) {
84+
t.Parallel()
85+
86+
got := handleUseCaseErr(tc.err, nil)
87+
require.Error(t, got)
88+
assert.Equal(t, tc.wantCode, status.Code(got))
89+
assert.Equal(t, tc.wantMessage, kerrors.FromError(got).GetMessage())
90+
})
91+
}
92+
}
93+
2794
func TestRequireCurrentUser(t *testing.T) {
2895
t.Parallel()
2996

0 commit comments

Comments
 (0)