featureflag: Remove `GetTreeEntriesStructuredErrors` flag

The flag `GetTreeEntriesStructuredErrors` was used to do a smooth
rollout of structured errors for the `GetTreeEntries` RPC.

The rollout was completed in the previous release of Gitaly, now we can
remove the flag and cleanup code around it.

3 files changed, 113 insertions, 200 deletions

diff --git a/internal/featureflag/ff_get_tree_entries_structured_errors.go b/internal/featureflag/ff_get_tree_entries_structured_errors.go
deleted file mode 100644
index 0bdf31b93..000000000
--- a/internal/featureflag/ff_get_tree_entries_structured_errors.go
+++ /dev/null
@@ -1,10 +0,0 @@
-package featureflag
-
-// GetTreeEntriesStructuredErrors enables the usage structured errors for
-// the GetTreeEntries RPC.
-var GetTreeEntriesStructuredErrors = NewFeatureFlag(
-	"get_tree_entries_structured_errors",
-	"v16.3.0",
-	"https://gitlab.com/gitlab-org/gitaly/-/issues/5157",
-	false,
-)
diff --git a/internal/gitaly/service/commit/get_tree_entries.go b/internal/gitaly/service/commit/get_tree_entries.go
index 765e2f141..13bcadcce 100644
--- a/internal/gitaly/service/commit/get_tree_entries.go
+++ b/internal/gitaly/service/commit/get_tree_entries.go
@@ -11,7 +11,6 @@ import (
 
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
 	log "github.com/sirupsen/logrus"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo"
@@ -26,7 +25,7 @@ const (
 	defaultFlatTreeRecursion = 10
 )
 
-func validateGetTreeEntriesRequest(locator storage.Locator, useStructuredErrors bool, in *gitalypb.GetTreeEntriesRequest) error {
+func validateGetTreeEntriesRequest(locator storage.Locator, in *gitalypb.GetTreeEntriesRequest) error {
 	if err := locator.ValidateRepository(in.GetRepository()); err != nil {
 		return structerr.NewInvalidArgument("%w", err)
 	}
@@ -35,17 +34,13 @@ func validateGetTreeEntriesRequest(locator storage.Locator, useStructuredErrors
 	}
 
 	if len(in.GetPath()) == 0 {
-		if useStructuredErrors {
-			return structerr.NewInvalidArgument("empty path").WithDetail(&gitalypb.GetTreeEntriesError{
-				Error: &gitalypb.GetTreeEntriesError_Path{
-					Path: &gitalypb.PathError{
-						ErrorType: gitalypb.PathError_ERROR_TYPE_EMPTY_PATH,
-					},
+		return structerr.NewInvalidArgument("empty path").WithDetail(&gitalypb.GetTreeEntriesError{
+			Error: &gitalypb.GetTreeEntriesError_Path{
+				Path: &gitalypb.PathError{
+					ErrorType: gitalypb.PathError_ERROR_TYPE_EMPTY_PATH,
 				},
-			})
-		}
-
-		return structerr.NewInvalidArgument("%w", fmt.Errorf("empty Path"))
+			},
+		})
 	}
 
 	return nil
@@ -95,8 +90,6 @@ func (s *server) sendTreeEntries(
 
 	var objectReader catfile.ObjectContentReader
 
-	useStructuredErrors := featureflag.GetTreeEntriesStructuredErrors.IsEnabled(ctx)
-
 	// While both repo.ReadTree and catfile.TreeEntries do this internally, in the case
 	// of non-recursive path, we do repo.ResolveRevision, which could fail because of this.
 	if path == "." {
@@ -127,43 +120,31 @@ func (s *server) sendTreeEntries(
 			}
 
 			if errors.Is(err, localrepo.ErrTreeNotExist) {
-				if useStructuredErrors {
-					return structerr.NewInvalidArgument("revision doesn't exist").WithDetail(&gitalypb.GetTreeEntriesError{
-						Error: &gitalypb.GetTreeEntriesError_ResolveTree{
-							ResolveTree: &gitalypb.ResolveRevisionError{
-								Revision: []byte(revision),
-							},
+				return structerr.NewInvalidArgument("revision doesn't exist").WithDetail(&gitalypb.GetTreeEntriesError{
+					Error: &gitalypb.GetTreeEntriesError_ResolveTree{
+						ResolveTree: &gitalypb.ResolveRevisionError{
+							Revision: []byte(revision),
 						},
-					}).WithMetadataItems(
-						structerr.MetadataItem{Key: "path", Value: path},
-						structerr.MetadataItem{Key: "revision", Value: revision},
-					)
-				}
-
-				// Previously rails only parsed empty response. This will be cleaned up
-				// with the rollout of structured errors.
-				return nil
+					},
+				}).WithMetadataItems(
+					structerr.MetadataItem{Key: "path", Value: path},
+					structerr.MetadataItem{Key: "revision", Value: revision},
+				)
 			}
 
 			if errors.Is(err, git.ErrReferenceNotFound) {
-				if useStructuredErrors {
-					// Since we rely on repo.ResolveRevision, it could either be an invalid revision
-					// or an invalid path.
-					return structerr.NewInvalidArgument("invalid revision or path").WithDetail(&gitalypb.GetTreeEntriesError{
-						Error: &gitalypb.GetTreeEntriesError_ResolveTree{
-							ResolveTree: &gitalypb.ResolveRevisionError{
-								Revision: []byte(revision),
-							},
+				// Since we rely on repo.ResolveRevision, it could either be an invalid revision
+				// or an invalid path.
+				return structerr.NewInvalidArgument("invalid revision or path").WithDetail(&gitalypb.GetTreeEntriesError{
+					Error: &gitalypb.GetTreeEntriesError_ResolveTree{
+						ResolveTree: &gitalypb.ResolveRevisionError{
+							Revision: []byte(revision),
 						},
-					}).WithMetadataItems(
-						structerr.MetadataItem{Key: "path", Value: path},
-						structerr.MetadataItem{Key: "revision", Value: revision},
-					)
-				}
-
-				// Previously rails only parsed empty response. This will be cleaned up
-				// with the rollout of structured errors.
-				return nil
+					},
+				}).WithMetadataItems(
+					structerr.MetadataItem{Key: "path", Value: path},
+					structerr.MetadataItem{Key: "revision", Value: revision},
+				)
 			}
 
 			return fmt.Errorf("reading tree: %w", err)
@@ -204,22 +185,18 @@ func (s *server) sendTreeEntries(
 		// we merge the two we can get rid of this check.
 		if _, err := repo.ResolveRevision(ctx, git.Revision(revision+":"+path)); err != nil {
 			if errors.Is(err, git.ErrReferenceNotFound) {
-				if useStructuredErrors {
-					// Since we rely on repo.ResolveRevision, it could either be an invalid revision
-					// or an invalid path.
-					return structerr.NewInvalidArgument("invalid revision or path").WithDetail(&gitalypb.GetTreeEntriesError{
-						Error: &gitalypb.GetTreeEntriesError_ResolveTree{
-							ResolveTree: &gitalypb.ResolveRevisionError{
-								Revision: []byte(revision),
-							},
+				// Since we rely on repo.ResolveRevision, it could either be an invalid revision
+				// or an invalid path.
+				return structerr.NewInvalidArgument("invalid revision or path").WithDetail(&gitalypb.GetTreeEntriesError{
+					Error: &gitalypb.GetTreeEntriesError_ResolveTree{
+						ResolveTree: &gitalypb.ResolveRevisionError{
+							Revision: []byte(revision),
 						},
-					}).WithMetadataItems(
-						structerr.MetadataItem{Key: "path", Value: path},
-						structerr.MetadataItem{Key: "revision", Value: revision},
-					)
-				}
-
-				return nil
+					},
+				}).WithMetadataItems(
+					structerr.MetadataItem{Key: "path", Value: path},
+					structerr.MetadataItem{Key: "revision", Value: revision},
+				)
 			}
 			return err
 		}
@@ -357,9 +334,7 @@ func (s *server) GetTreeEntries(in *gitalypb.GetTreeEntriesRequest, stream gital
 		"Path":     in.Path,
 	}).Debug("GetTreeEntries")
 
-	useStructuredErrors := featureflag.GetTreeEntriesStructuredErrors.IsEnabled(stream.Context())
-
-	if err := validateGetTreeEntriesRequest(s.locator, useStructuredErrors, in); err != nil {
+	if err := validateGetTreeEntriesRequest(s.locator, in); err != nil {
 		return err
 	}
 
diff --git a/internal/gitaly/service/commit/get_tree_entries_test.go b/internal/gitaly/service/commit/get_tree_entries_test.go
index 533ffbb40..203704947 100644
--- a/internal/gitaly/service/commit/get_tree_entries_test.go
+++ b/internal/gitaly/service/commit/get_tree_entries_test.go
@@ -1,14 +1,12 @@
 package commit
 
 import (
-	"context"
 	"fmt"
 	"io"
 	"strconv"
 	"testing"
 
 	"github.com/stretchr/testify/require"
-	"gitlab.com/gitlab-org/gitaly/v16/internal/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config"
@@ -26,12 +24,7 @@ import (
 func TestGetTreeEntries(t *testing.T) {
 	t.Parallel()
 
-	testhelper.NewFeatureSets(featureflag.GetTreeEntriesStructuredErrors).Run(t, testGetTreeEntries)
-}
-
-func testGetTreeEntries(t *testing.T, ctx context.Context) {
-	t.Parallel()
-
+	ctx := testhelper.Context(t)
 	cfg := testcfg.Build(t)
 
 	cfg.SocketPath = startTestServices(t, cfg)
@@ -174,23 +167,18 @@ func testGetTreeEntries(t *testing.T, ctx context.Context) {
 					}),
 				}))
 
-				var expectedErr error = structerr.NewInvalidArgument("empty Path")
-				if featureflag.GetTreeEntriesStructuredErrors.IsEnabled(ctx) {
-					expectedErr = structerr.NewInvalidArgument("empty path").WithDetail(&gitalypb.GetTreeEntriesError{
-						Error: &gitalypb.GetTreeEntriesError_Path{
-							Path: &gitalypb.PathError{
-								ErrorType: gitalypb.PathError_ERROR_TYPE_EMPTY_PATH,
-							},
-						},
-					})
-				}
-
 				return setupData{
 					request: &gitalypb.GetTreeEntriesRequest{
 						Repository: repo,
 						Revision:   []byte(commitID),
 					},
-					expectedErr: expectedErr,
+					expectedErr: structerr.NewInvalidArgument("empty path").WithDetail(&gitalypb.GetTreeEntriesError{
+						Error: &gitalypb.GetTreeEntriesError_Path{
+							Path: &gitalypb.PathError{
+								ErrorType: gitalypb.PathError_ERROR_TYPE_EMPTY_PATH,
+							},
+						},
+					}),
 				}
 			},
 		},
@@ -270,9 +258,13 @@ func testGetTreeEntries(t *testing.T, ctx context.Context) {
 					}),
 				)
 
-				var expectedErr error
-				if featureflag.GetTreeEntriesStructuredErrors.IsEnabled(ctx) {
-					expectedErr = testhelper.WithInterceptedMetadataItems(
+				return setupData{
+					request: &gitalypb.GetTreeEntriesRequest{
+						Repository: repo,
+						Revision:   []byte(commitID.String()),
+						Path:       []byte("./.."),
+					},
+					expectedErr: testhelper.WithInterceptedMetadataItems(
 						structerr.NewInvalidArgument("invalid revision or path").WithDetail(&gitalypb.GetTreeEntriesError{
 							Error: &gitalypb.GetTreeEntriesError_ResolveTree{
 								ResolveTree: &gitalypb.ResolveRevisionError{
@@ -282,16 +274,7 @@ func testGetTreeEntries(t *testing.T, ctx context.Context) {
 						}),
 						structerr.MetadataItem{Key: "path", Value: "./.."},
 						structerr.MetadataItem{Key: "revision", Value: commitID},
-					)
-				}
-
-				return setupData{
-					request: &gitalypb.GetTreeEntriesRequest{
-						Repository: repo,
-						Revision:   []byte(commitID.String()),
-						Path:       []byte("./.."),
-					},
-					expectedErr: expectedErr,
+					),
 				}
 			},
 		},
@@ -306,9 +289,13 @@ func testGetTreeEntries(t *testing.T, ctx context.Context) {
 					}),
 				}))
 
-				var expectedErr error
-				if featureflag.GetTreeEntriesStructuredErrors.IsEnabled(ctx) {
-					expectedErr = testhelper.WithInterceptedMetadataItems(
+				return setupData{
+					request: &gitalypb.GetTreeEntriesRequest{
+						Repository: repo,
+						Revision:   []byte(commitID.String()),
+						Path:       []byte("./folder/.."),
+					},
+					expectedErr: testhelper.WithInterceptedMetadataItems(
 						structerr.NewInvalidArgument("invalid revision or path").WithDetail(&gitalypb.GetTreeEntriesError{
 							Error: &gitalypb.GetTreeEntriesError_ResolveTree{
 								ResolveTree: &gitalypb.ResolveRevisionError{
@@ -318,16 +305,7 @@ func testGetTreeEntries(t *testing.T, ctx context.Context) {
 						}),
 						structerr.MetadataItem{Key: "path", Value: "./folder/.."},
 						structerr.MetadataItem{Key: "revision", Value: commitID},
-					)
-				}
-
-				return setupData{
-					request: &gitalypb.GetTreeEntriesRequest{
-						Repository: repo,
-						Revision:   []byte(commitID.String()),
-						Path:       []byte("./folder/.."),
-					},
-					expectedErr: expectedErr,
+					),
 				}
 			},
 		},
@@ -342,9 +320,13 @@ func testGetTreeEntries(t *testing.T, ctx context.Context) {
 					}),
 				}))
 
-				var expectedErr error
-				if featureflag.GetTreeEntriesStructuredErrors.IsEnabled(ctx) {
-					expectedErr = testhelper.WithInterceptedMetadataItems(
+				return setupData{
+					request: &gitalypb.GetTreeEntriesRequest{
+						Repository: repo,
+						Revision:   []byte(commitID.String()),
+						Path:       []byte("./folder/test.txt"),
+					},
+					expectedErr: testhelper.WithInterceptedMetadataItems(
 						structerr.NewInvalidArgument("invalid revision or path").WithDetail(&gitalypb.GetTreeEntriesError{
 							Error: &gitalypb.GetTreeEntriesError_ResolveTree{
 								ResolveTree: &gitalypb.ResolveRevisionError{
@@ -354,16 +336,7 @@ func testGetTreeEntries(t *testing.T, ctx context.Context) {
 						}),
 						structerr.MetadataItem{Key: "path", Value: "./folder/test.txt"},
 						structerr.MetadataItem{Key: "revision", Value: commitID},
-					)
-				}
-
-				return setupData{
-					request: &gitalypb.GetTreeEntriesRequest{
-						Repository: repo,
-						Revision:   []byte(commitID.String()),
-						Path:       []byte("./folder/test.txt"),
-					},
-					expectedErr: expectedErr,
+					),
 				}
 			},
 		},
@@ -451,9 +424,13 @@ func testGetTreeEntries(t *testing.T, ctx context.Context) {
 					Path: "folder",
 				}))
 
-				var expectedErr error
-				if featureflag.GetTreeEntriesStructuredErrors.IsEnabled(ctx) {
-					expectedErr = testhelper.WithInterceptedMetadataItems(
+				return setupData{
+					request: &gitalypb.GetTreeEntriesRequest{
+						Repository: repo,
+						Revision:   []byte(commitID.String()),
+						Path:       []byte(repoPath + "folder"),
+					},
+					expectedErr: testhelper.WithInterceptedMetadataItems(
 						structerr.NewInvalidArgument("invalid revision or path").WithDetail(&gitalypb.GetTreeEntriesError{
 							Error: &gitalypb.GetTreeEntriesError_ResolveTree{
 								ResolveTree: &gitalypb.ResolveRevisionError{
@@ -463,16 +440,7 @@ func testGetTreeEntries(t *testing.T, ctx context.Context) {
 						}),
 						structerr.MetadataItem{Key: "path", Value: repoPath + "folder"},
 						structerr.MetadataItem{Key: "revision", Value: commitID},
-					)
-				}
-
-				return setupData{
-					request: &gitalypb.GetTreeEntriesRequest{
-						Repository: repo,
-						Revision:   []byte(commitID.String()),
-						Path:       []byte(repoPath + "folder"),
-					},
-					expectedErr: expectedErr,
+					),
 				}
 			},
 		},
@@ -846,9 +814,13 @@ func testGetTreeEntries(t *testing.T, ctx context.Context) {
 					gittest.TreeEntry{OID: folderOID, Mode: "040000", Path: "foo"},
 				))
 
-				var expectedErr error
-				if featureflag.GetTreeEntriesStructuredErrors.IsEnabled(ctx) {
-					expectedErr = testhelper.WithInterceptedMetadataItems(
+				return setupData{
+					request: &gitalypb.GetTreeEntriesRequest{
+						Repository: repo,
+						Revision:   []byte(commitID),
+						Path:       []byte("does-not-exist"),
+					},
+					expectedErr: testhelper.WithInterceptedMetadataItems(
 						structerr.NewInvalidArgument("invalid revision or path").WithDetail(&gitalypb.GetTreeEntriesError{
 							Error: &gitalypb.GetTreeEntriesError_ResolveTree{
 								ResolveTree: &gitalypb.ResolveRevisionError{
@@ -858,16 +830,7 @@ func testGetTreeEntries(t *testing.T, ctx context.Context) {
 						}),
 						structerr.MetadataItem{Key: "path", Value: "does-not-exist"},
 						structerr.MetadataItem{Key: "revision", Value: commitID},
-					)
-				}
-
-				return setupData{
-					request: &gitalypb.GetTreeEntriesRequest{
-						Repository: repo,
-						Revision:   []byte(commitID),
-						Path:       []byte("does-not-exist"),
-					},
-					expectedErr: expectedErr,
+					),
 				}
 			},
 		},
@@ -886,9 +849,14 @@ func testGetTreeEntries(t *testing.T, ctx context.Context) {
 					gittest.TreeEntry{OID: folderOID, Mode: "040000", Path: "foo"},
 				))
 
-				var expectedErr error
-				if featureflag.GetTreeEntriesStructuredErrors.IsEnabled(ctx) {
-					expectedErr = testhelper.WithInterceptedMetadataItems(
+				return setupData{
+					request: &gitalypb.GetTreeEntriesRequest{
+						Repository: repo,
+						Revision:   []byte(commitID),
+						Path:       []byte("does-not-exist"),
+						Recursive:  true,
+					},
+					expectedErr: testhelper.WithInterceptedMetadataItems(
 						structerr.NewInvalidArgument("invalid revision or path").WithDetail(&gitalypb.GetTreeEntriesError{
 							Error: &gitalypb.GetTreeEntriesError_ResolveTree{
 								ResolveTree: &gitalypb.ResolveRevisionError{
@@ -898,17 +866,7 @@ func testGetTreeEntries(t *testing.T, ctx context.Context) {
 						}),
 						structerr.MetadataItem{Key: "path", Value: "does-not-exist"},
 						structerr.MetadataItem{Key: "revision", Value: commitID},
-					)
-				}
-
-				return setupData{
-					request: &gitalypb.GetTreeEntriesRequest{
-						Repository: repo,
-						Revision:   []byte(commitID),
-						Path:       []byte("does-not-exist"),
-						Recursive:  true,
-					},
-					expectedErr: expectedErr,
+					),
 				}
 			},
 		},
@@ -927,9 +885,13 @@ func testGetTreeEntries(t *testing.T, ctx context.Context) {
 					gittest.TreeEntry{OID: folderOID, Mode: "040000", Path: "foo"},
 				))
 
-				var expectedErr error
-				if featureflag.GetTreeEntriesStructuredErrors.IsEnabled(ctx) {
-					expectedErr = testhelper.WithInterceptedMetadataItems(
+				return setupData{
+					request: &gitalypb.GetTreeEntriesRequest{
+						Repository: repo,
+						Revision:   []byte("does-not-exist"),
+						Path:       []byte("."),
+					},
+					expectedErr: testhelper.WithInterceptedMetadataItems(
 						structerr.NewInvalidArgument("invalid revision or path").WithDetail(&gitalypb.GetTreeEntriesError{
 							Error: &gitalypb.GetTreeEntriesError_ResolveTree{
 								ResolveTree: &gitalypb.ResolveRevisionError{
@@ -939,16 +901,7 @@ func testGetTreeEntries(t *testing.T, ctx context.Context) {
 						}),
 						structerr.MetadataItem{Key: "path", Value: ""},
 						structerr.MetadataItem{Key: "revision", Value: "does-not-exist"},
-					)
-				}
-
-				return setupData{
-					request: &gitalypb.GetTreeEntriesRequest{
-						Repository: repo,
-						Revision:   []byte("does-not-exist"),
-						Path:       []byte("."),
-					},
-					expectedErr: expectedErr,
+					),
 				}
 			},
 		},
@@ -967,9 +920,14 @@ func testGetTreeEntries(t *testing.T, ctx context.Context) {
 					gittest.TreeEntry{OID: folderOID, Mode: "040000", Path: "foo"},
 				))
 
-				var expectedErr error
-				if featureflag.GetTreeEntriesStructuredErrors.IsEnabled(ctx) {
-					expectedErr = testhelper.WithInterceptedMetadataItems(
+				return setupData{
+					request: &gitalypb.GetTreeEntriesRequest{
+						Repository: repo,
+						Revision:   []byte("does-not-exist"),
+						Path:       []byte("."),
+						Recursive:  true,
+					},
+					expectedErr: testhelper.WithInterceptedMetadataItems(
 						structerr.NewInvalidArgument("invalid revision or path").WithDetail(&gitalypb.GetTreeEntriesError{
 							Error: &gitalypb.GetTreeEntriesError_ResolveTree{
 								ResolveTree: &gitalypb.ResolveRevisionError{
@@ -979,17 +937,7 @@ func testGetTreeEntries(t *testing.T, ctx context.Context) {
 						}),
 						structerr.MetadataItem{Key: "path", Value: ""},
 						structerr.MetadataItem{Key: "revision", Value: "does-not-exist"},
-					)
-				}
-
-				return setupData{
-					request: &gitalypb.GetTreeEntriesRequest{
-						Repository: repo,
-						Revision:   []byte("does-not-exist"),
-						Path:       []byte("."),
-						Recursive:  true,
-					},
-					expectedErr: expectedErr,
+					),
 				}
 			},
 		},