Fix CreatePipeline retry (#6859)

server/store/datastore/helper.go

@@ -53,8 +53,11 @@ func wrapDelete(c int64, err error) error {
 
 func wrapInsert(c int64, err error) error {
 	if err != nil {
-		if errMsg := err.Error(); strings.HasPrefix(errMsg, "UNIQUE constraint failed") ||
-			strings.HasPrefix(errMsg, "pq: duplicate key value violates unique constraint") ||
+		// Common unique constraint violation patterns across the supported drivers.
+		if errMsg := err.Error(); strings.Contains(errMsg, "UNIQUE constraint failed") ||
+			strings.Contains(errMsg, "UNIQUE violation") ||
+			strings.Contains(errMsg, "duplicate key") ||
+			strings.Contains(errMsg, "unique constraint") ||
 			strings.Contains(errMsg, "Duplicate entry") {
 			return types.ErrInsertDuplicateDetected
 		}

server/store/datastore/helper_test.go

@@ -50,4 +50,20 @@ func TestWrapInsert(t *testing.T) {
 
 	// test insert witch should fail because of unique constraint
 	assert.ErrorIs(t, wrapInsert(store.engine.Insert(cron)), types.ErrInsertDuplicateDetected)
+
+	// The store above only exercises the sqlite wording. Cover the other drivers too: callers rely on
+	// errors.Is(err, ErrInsertDuplicateDetected) instead of matching driver strings themselves, so a
+	// pattern missing here silently breaks them (e.g. the retry in CreatePipeline, #6067).
+	for _, driverErr := range []string{
+		`pq: duplicate key value violates unique constraint "UQE_pipelines_s"`, // postgres
+		"Error 1062: Duplicate entry '1-2' for key 'UQE_pipelines_s'",          // mysql
+		"UNIQUE violation",
+		"unique constraint",
+	} {
+		assert.ErrorIs(t, wrapInsert(0, errors.New(driverErr)), types.ErrInsertDuplicateDetected, driverErr)
+	}
+
+	// Everything else passes through untouched.
+	other := errors.New("connection refused")
+	assert.Equal(t, other, wrapInsert(0, other))
 }

server/store/datastore/pipeline.go

@@ -16,7 +16,7 @@ package datastore
 
 import (
 	"context"
-	"strings"
+	"errors"
 	"time"
 
 	"github.com/cenkalti/backoff/v7"
@@ -24,6 +24,7 @@ import (
 	"xorm.io/xorm"
 
 	"go.woodpecker-ci.org/woodpecker/v3/server/model"
+	"go.woodpecker-ci.org/woodpecker/v3/server/store/types"
 )
 
 func (s storage) GetPipeline(id int64) (*model.Pipeline, error) {
@@ -169,7 +170,7 @@ func (s storage) CreatePipeline(pipeline *model.Pipeline, stepList ...*model.Ste
 		pipeline.Created = time.Now().UTC().Unix()
 		// only Insert set auto created ID back to object
 		if err := wrapInsert(sess.Insert(pipeline)); err != nil {
-			if isUniqueConstraintError(err) {
+			if errors.Is(err, types.ErrInsertDuplicateDetected) {
 				return struct{}{}, err
 			}
 			return struct{}{}, backoff.Permanent(err)
@@ -179,7 +180,7 @@ func (s storage) CreatePipeline(pipeline *model.Pipeline, stepList ...*model.Ste
 			stepList[i].PipelineID = pipeline.ID
 			// only Insert set auto created ID back to object
 			if err := wrapInsert(sess.Insert(stepList[i])); err != nil {
-				if isUniqueConstraintError(err) {
+				if errors.Is(err, types.ErrInsertDuplicateDetected) {
 					return struct{}{}, err
 				}
 				return struct{}{}, backoff.Permanent(err)
@@ -192,21 +193,6 @@ func (s storage) CreatePipeline(pipeline *model.Pipeline, stepList ...*model.Ste
 	return err
 }
 
-// isUniqueConstraintError checks if an error is a unique constraint violation error.
-func isUniqueConstraintError(err error) bool {
-	if err == nil {
-		return false
-	}
-
-	errStr := err.Error()
-	// Check for common unique constraint error patterns across different databases
-	return strings.Contains(errStr, "duplicate key") ||
-		strings.Contains(errStr, "Duplicate entry") ||
-		strings.Contains(errStr, "UNIQUE constraint failed") ||
-		strings.Contains(errStr, "unique constraint") ||
-		strings.Contains(errStr, "UNIQUE violation")
-}
-
 func (s storage) UpdatePipeline(pipeline *model.Pipeline) error {
 	_, err := s.engine.ID(pipeline.ID).AllCols().Update(pipeline)
 	return err

server/store/datastore/pipeline_test.go

@@ -218,6 +218,31 @@ func TestPipelineIncrement(t *testing.T) {
 	assert.EqualValues(t, 1, pipelineC.Number)
 }
 
+// Duplicates must surface as ErrInsertDuplicateDetected so the retry sees them as retryable (#6067).
+func TestCreatePipelineDuplicateIsRetryable(t *testing.T) {
+	store, closer := newTestStore(t, new(model.Repo), new(model.Step), new(model.Pipeline))
+	defer closer()
+
+	require.NoError(t, store.CreateRepo(&model.Repo{ID: 1, Owner: "1", Name: "1", FullName: "1/1", ForgeRemoteID: "1"}))
+
+	// Duplicate step: UNIQUE(pipeline_id, pid).
+	err := store.CreatePipeline(&model.Pipeline{RepoID: 1},
+		&model.Step{PID: 1, Name: "clone"},
+		&model.Step{PID: 1, Name: "dup"},
+	)
+	assert.ErrorIs(t, err, types.ErrInsertDuplicateDetected)
+
+	// Every attempt rolled back.
+	count, err := store.GetPipelineCount()
+	assert.NoError(t, err)
+	assert.Zero(t, count)
+
+	// Same for the pipeline insert. Forced through the PK: the number is MAX+1, it can't collide on demand.
+	existing := &model.Pipeline{RepoID: 1}
+	require.NoError(t, store.CreatePipeline(existing))
+	assert.ErrorIs(t, store.CreatePipeline(&model.Pipeline{ID: existing.ID, RepoID: 1}), types.ErrInsertDuplicateDetected)
+}
+
 func TestDeletePipeline(t *testing.T) {
 	store, closer := newTestStore(t, new(model.Pipeline), new(model.Repo), new(model.Workflow),
 		new(model.Step), new(model.LogEntry), new(model.PipelineConfig), new(model.Config))