From 264bd5195e761ef13f457852805b09109f35476b Mon Sep 17 00:00:00 2001 From: Steve Azzopardi Date: Fri, 22 Jul 2022 08:51:04 +0200 Subject: Change FindTag err when tag not found to NotFound What --- - Update the error message to be `Not Found` rather then `Internal` when a tag is not found behind a feature flag. Why --- We burn through our error budget when a single user sends a bunch of requests to tags that don't exist because we classify the error as `Internal`, instead it should be `NotFound`. We need to set this behind a feature flag to prevent any deployment issues because the [rails application](https://bit.ly/3onH0x6) expects `Internal` err. In https://gitlab.com/gitlab-org/gitlab/-/merge_requests/93009 we are adding support for `NotFound`. Changelog: changed Reference: https://gitlab.com/gitlab-org/gitaly/-/issues/4366 Signed-off-by: Steve Azzopardi --- internal/gitaly/service/ref/refs.go | 4 ++++ internal/gitaly/service/ref/refs_test.go | 20 ++++++++++++++++++++ .../metadata/featureflag/ff_find_tag_not_found.go | 10 ++++++++++ 3 files changed, 34 insertions(+) create mode 100644 internal/metadata/featureflag/ff_find_tag_not_found.go diff --git a/internal/gitaly/service/ref/refs.go b/internal/gitaly/service/ref/refs.go index 19e532dc0..2fe6cf4a8 100644 --- a/internal/gitaly/service/ref/refs.go +++ b/internal/gitaly/service/ref/refs.go @@ -257,6 +257,10 @@ func (s *server) findTag(ctx context.Context, repo git.RepositoryExecutor, tagNa return nil, err } } else { + if featureflag.FindTagNotFound.IsEnabled(ctx) { + return nil, helper.ErrNotFoundf("no tag found") + } + return nil, errors.New("no tag found") } diff --git a/internal/gitaly/service/ref/refs_test.go b/internal/gitaly/service/ref/refs_test.go index aa4e0faff..5f3ec6b38 100644 --- a/internal/gitaly/service/ref/refs_test.go +++ b/internal/gitaly/service/ref/refs_test.go @@ -1212,3 +1212,23 @@ func TestInvalidFindTagRequest(t *testing.T) { }) } } + +func TestNotFoundFindTagRequest(t *testing.T) { + t.Parallel() + testhelper.NewFeatureSets(featureflag.FindTagNotFound).Run(t, testNotFoundFindTagRequest) +} + +func testNotFoundFindTagRequest(t *testing.T, ctx context.Context) { + t.Parallel() + + _, repoProto, _, client := setupRefService(ctx, t) + + rpcRequest := &gitalypb.FindTagRequest{Repository: repoProto, TagName: []byte("not-found")} + + _, err := client.FindTag(ctx, rpcRequest) + if featureflag.FindTagNotFound.IsEnabled(ctx) { + testhelper.RequireGrpcError(t, err, helper.ErrNotFoundf("no tag found")) + return + } + testhelper.RequireGrpcError(t, err, helper.ErrInternalf("no tag found")) +} diff --git a/internal/metadata/featureflag/ff_find_tag_not_found.go b/internal/metadata/featureflag/ff_find_tag_not_found.go new file mode 100644 index 000000000..c00653118 --- /dev/null +++ b/internal/metadata/featureflag/ff_find_tag_not_found.go @@ -0,0 +1,10 @@ +package featureflag + +// FindTagNotFound will return NotFound instead of Internal when the Git Tag +// doesn't exist. +var FindTagNotFound = NewFeatureFlag( + "find_tag_not_found", + "v15.2.0", + "https://gitlab.com/gitlab-org/gitaly/-/issues/4366", + false, +) -- cgit v1.2.3