diff --git a/server/api/helper.go b/server/api/helper.go index 5103b67ed2..19c3653169 100644 --- a/server/api/helper.go +++ b/server/api/helper.go @@ -30,12 +30,14 @@ import ( ) func handlePipelineErr(c *gin.Context, err error) { - if errors.Is(err, &pipeline.ErrNotFound{}) { + var notFound *pipeline.ErrNotFound + var badRequest *pipeline.ErrBadRequest + if errors.As(err, ¬Found) { c.String(http.StatusNotFound, "%s", err) - } else if errors.Is(err, &pipeline.ErrBadRequest{}) { + } else if errors.As(err, &badRequest) { c.String(http.StatusBadRequest, "%s", err) - } else if errors.Is(err, &pipeline.ErrFiltered{}) { - c.Status(http.StatusNoContent) + } else if errors.Is(err, pipeline.ErrFilteredRestrictions) || errors.Is(err, pipeline.ErrFilteredSteps) { + c.String(http.StatusNoContent, "%s", err) } else { _ = c.AbortWithError(http.StatusInternalServerError, err) } diff --git a/server/api/helper_test.go b/server/api/helper_test.go new file mode 100644 index 0000000000..b820f43a6c --- /dev/null +++ b/server/api/helper_test.go @@ -0,0 +1,45 @@ +package api + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + + "github.com/woodpecker-ci/woodpecker/server/pipeline" +) + +func TestHandlePipelineError(t *testing.T) { + tests := []struct { + err error + code int + }{ + { + err: pipeline.ErrFilteredRestrictions, + code: http.StatusNoContent, + }, + { + err: pipeline.ErrFilteredSteps, + code: http.StatusNoContent, + }, + { + err: &pipeline.ErrNotFound{Msg: "pipeline not found"}, + code: http.StatusNotFound, + }, + { + err: &pipeline.ErrBadRequest{Msg: "bad request error"}, + code: http.StatusBadRequest, + }, + } + + for _, tt := range tests { + r := httptest.NewRecorder() + c, _ := gin.CreateTestContext(r) + handlePipelineErr(c, tt.err) + if r.Code != tt.code { + t.Errorf("status code: %d - expected: %d", r.Code, tt.code) + } + } + +} diff --git a/server/pipeline/create.go b/server/pipeline/create.go index f4ac0864d0..f538189186 100644 --- a/server/pipeline/create.go +++ b/server/pipeline/create.go @@ -66,13 +66,13 @@ func Create(ctx context.Context, _store store.Store, repo *model.Repo, pipeline filtered, parseErr = checkIfFiltered(repo, pipeline, forgeYamlConfigs) if parseErr == nil { if filtered { - err := ErrFiltered{Msg: "global when filter of all workflows do skip this pipeline"} + err := ErrFilteredRestrictions log.Debug().Str("repo", repo.FullName).Msgf("%v", err) return nil, err } if zeroSteps(pipeline, forgeYamlConfigs) { - err := ErrFiltered{Msg: "step conditions yield zero runnable steps"} + err := ErrFilteredSteps log.Debug().Str("repo", repo.FullName).Msgf("%v", err) return nil, err } diff --git a/server/pipeline/errors.go b/server/pipeline/errors.go index e9f2826f51..3cc297742c 100644 --- a/server/pipeline/errors.go +++ b/server/pipeline/errors.go @@ -14,6 +14,8 @@ package pipeline +import "errors" + type ErrNotFound struct { Msg string } @@ -38,26 +40,7 @@ func (e ErrBadRequest) Error() string { return e.Msg } -func (e ErrBadRequest) Is(target error) bool { - _, ok := target.(ErrBadRequest) //nolint:errorlint - if !ok { - _, ok = target.(*ErrBadRequest) //nolint:errorlint - } - return ok -} - -type ErrFiltered struct { - Msg string -} - -func (e ErrFiltered) Error() string { - return "ignoring hook: " + e.Msg -} - -func (e *ErrFiltered) Is(target error) bool { - _, ok := target.(ErrFiltered) //nolint:errorlint - if !ok { - _, ok = target.(*ErrFiltered) //nolint:errorlint - } - return ok -} +var ( + ErrFilteredRestrictions = errors.New("ignoring hook: branch does not match restrictions defined in yaml") // global when filter of all workflows do skip this pipeline + ErrFilteredSteps = errors.New("ignoring hook: step conditions yield zero runnable steps") +)