From edd9b6066c3ef8045c1c8a46b1d19bd44bae1981 Mon Sep 17 00:00:00 2001
From: John Cai <jcai@gitlab.com>
Date: Wed, 28 Sep 2022 13:16:39 -0400
Subject: ref: Return structured error when update error is not
 ErrAlreadyLocked

We have a bug wherein we still return a successful response with an
error embedded in the response if the Prepare() step fails with an error
but the error is not an ErrAlreadyLocked. Fix the code such that we
return an error instead of a response when there is a non
ErrAlreadyLocked error during the Prepare() call.

Changelog: fixed
---
 internal/gitaly/service/ref/delete_refs.go | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/internal/gitaly/service/ref/delete_refs.go b/internal/gitaly/service/ref/delete_refs.go
index f8d7a7f03..e6931a121 100644
--- a/internal/gitaly/service/ref/delete_refs.go
+++ b/internal/gitaly/service/ref/delete_refs.go
@@ -84,23 +84,26 @@ func (s *server) DeleteRefs(ctx context.Context, in *gitalypb.DeleteRefsRequest)
 
 	if err := updater.Prepare(); err != nil {
 		var errAlreadyLocked *updateref.ErrAlreadyLocked
-		if featureflag.DeleteRefsStructuredErrors.IsEnabled(ctx) &&
-			errors.As(err, &errAlreadyLocked) {
-			detailedErr, err := helper.ErrWithDetails(
-				helper.ErrFailedPreconditionf("cannot lock references"),
-				&gitalypb.DeleteRefsError{
-					Error: &gitalypb.DeleteRefsError_ReferencesLocked{
-						ReferencesLocked: &gitalypb.ReferencesLockedError{
-							Refs: [][]byte{[]byte(errAlreadyLocked.Ref)},
+		if featureflag.DeleteRefsStructuredErrors.IsEnabled(ctx) {
+			if errors.As(err, &errAlreadyLocked) {
+				detailedErr, err := helper.ErrWithDetails(
+					helper.ErrFailedPreconditionf("cannot lock references"),
+					&gitalypb.DeleteRefsError{
+						Error: &gitalypb.DeleteRefsError_ReferencesLocked{
+							ReferencesLocked: &gitalypb.ReferencesLockedError{
+								Refs: [][]byte{[]byte(errAlreadyLocked.Ref)},
+							},
 						},
 					},
-				},
-			)
-			if err != nil {
-				return nil, helper.ErrInternalf("error details: %w", err)
+				)
+				if err != nil {
+					return nil, helper.ErrInternalf("error details: %w", err)
+				}
+
+				return nil, detailedErr
 			}
 
-			return nil, detailedErr
+			return nil, helper.ErrInternalf("unable to prepare: %w", err)
 		}
 
 		return &gitalypb.DeleteRefsResponse{
-- 
cgit v1.2.3