Fix to show build pipeline parse error (#1066)

pipeline/frontend/yaml/error.go

@@ -0,0 +1,30 @@
+// Copyright 2022 Woodpecker Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package yaml
+
+// PipelineParseError is an error that occurs when the pipeline parsing fails.
+type PipelineParseError struct {
+	Err error
+}
+
+func (e PipelineParseError) Error() string {
+	return e.Err.Error()
+}
+
+func (e PipelineParseError) Is(target error) bool {
+	_, ok1 := target.(PipelineParseError)
+	_, ok2 := target.(*PipelineParseError)
+	return ok1 || ok2
+}

pipeline/frontend/yaml/matrix/matrix.go

@@ -3,6 +3,8 @@ package matrix
 import (
 	"strings"
 
+	pipeline "github.com/woodpecker-ci/woodpecker/pipeline/frontend/yaml"
+
 	"gopkg.in/yaml.v3"
 )
 
@@ -99,8 +101,10 @@ func parse(raw []byte) (Matrix, error) {
 	data := struct {
 		Matrix map[string][]string
 	}{}
-	err := yaml.Unmarshal(raw, &data)
-	return data.Matrix, err
+	if err := yaml.Unmarshal(raw, &data); err != nil {
+		return nil, &pipeline.PipelineParseError{Err: err}
+	}
+	return data.Matrix, nil
 }
 
 func parseList(raw []byte) ([]Axis, error) {
@@ -110,6 +114,8 @@ func parseList(raw []byte) ([]Axis, error) {
 		}
 	}{}
 
-	err := yaml.Unmarshal(raw, &data)
-	return data.Matrix.Include, err
+	if err := yaml.Unmarshal(raw, &data); err != nil {
+		return nil, &pipeline.PipelineParseError{Err: err}
+	}
+	return data.Matrix.Include, nil
 }

server/pipeline/create.go

@@ -17,6 +17,7 @@ package pipeline
 import (
 	"context"
 	"fmt"
+	"time"
 
 	"github.com/rs/zerolog/log"
 
@@ -51,40 +52,52 @@ func Create(ctx context.Context, _store store.Store, repo *model.Repo, build *mo
 		}
 	}
 
+	var (
+		remoteYamlConfigs []*remote.FileMeta
+		configFetchErr    error
+		filtered          bool
+		parseErr          error
+	)
+
 	// fetch the build file from the remote
 	configFetcher := shared.NewConfigFetcher(server.Config.Services.Remote, server.Config.Services.ConfigService, repoUser, repo, build)
-	remoteYamlConfigs, err := configFetcher.Fetch(ctx)
-	if err != nil {
-		msg := fmt.Sprintf("cannot find config '%s' in '%s' with user: '%s'", repo.Config, build.Ref, repoUser.Login)
-		log.Debug().Err(err).Str("repo", repo.FullName).Msg(msg)
-		return nil, ErrNotFound{Msg: msg}
-	}
-
-	filtered, err := branchFiltered(build, remoteYamlConfigs)
-	if err != nil {
-		msg := "failure to parse yaml from hook"
-		log.Debug().Err(err).Str("repo", repo.FullName).Msg(msg)
-		return nil, ErrBadRequest{Msg: msg}
-	}
-	if filtered {
-		err := ErrFiltered{Msg: "branch does not match restrictions defined in yaml"}
-		log.Debug().Str("repo", repo.FullName).Msgf("%v", err)
-		return nil, err
-	}
+	remoteYamlConfigs, configFetchErr = configFetcher.Fetch(ctx)
+	if configFetchErr == nil {
+		filtered, parseErr = branchFiltered(build, remoteYamlConfigs)
+		if parseErr == nil {
+			if filtered {
+				err := ErrFiltered{Msg: "branch does not match restrictions defined in yaml"}
+				log.Debug().Str("repo", repo.FullName).Msgf("%v", err)
+				return nil, err
+			}
 
-	if zeroSteps(build, remoteYamlConfigs) {
-		err := ErrFiltered{Msg: "step conditions yield zero runnable steps"}
-		log.Debug().Str("repo", repo.FullName).Msgf("%v", err)
-		return nil, err
+			if zeroSteps(build, remoteYamlConfigs) {
+				err := ErrFiltered{Msg: "step conditions yield zero runnable steps"}
+				log.Debug().Str("repo", repo.FullName).Msgf("%v", err)
+				return nil, err
+			}
+		}
 	}
 
 	// update some build fields
 	build.RepoID = repo.ID
 	build.Verified = true
 	build.Status = model.StatusPending
 
-	// TODO(336) extend gated feature with an allow/block List
-	if repo.IsGated {
+	if configFetchErr != nil {
+		log.Debug().Str("repo", repo.FullName).Err(configFetchErr).Msgf("cannot find config '%s' in '%s' with user: '%s'", repo.Config, build.Ref, repoUser.Login)
+		build.Started = time.Now().Unix()
+		build.Finished = build.Started
+		build.Status = model.StatusError
+		build.Error = fmt.Sprintf("pipeline definition not found in %s", repo.FullName)
+	} else if parseErr != nil {
+		log.Debug().Str("repo", repo.FullName).Err(parseErr).Msg("failed to parse yaml")
+		build.Started = time.Now().Unix()
+		build.Finished = build.Started
+		build.Status = model.StatusError
+		build.Error = fmt.Sprintf("failed to parse pipeline: %s", parseErr.Error())
+	} else if repo.IsGated {
+		// TODO(336) extend gated feature with an allow/block List
 		build.Status = model.StatusBlocked
 	}
 
@@ -105,6 +118,18 @@ func Create(ctx context.Context, _store store.Store, repo *model.Repo, build *mo
 		}
 	}
 
+	if build.Status == model.StatusError {
+		if err := publishToTopic(ctx, build, repo); err != nil {
+			log.Error().Err(err).Msg("publishToTopic")
+		}
+
+		if err := updateBuildStatus(ctx, build, repo, repoUser); err != nil {
+			log.Error().Err(err).Msg("updateBuildStatus")
+		}
+
+		return build, nil
+	}
+
 	build, buildItems, err := createBuildItems(ctx, _store, build, repoUser, repo, remoteYamlConfigs, nil)
 	if err != nil {
 		msg := fmt.Sprintf("failure to createBuildItems for %s", repo.FullName)

server/pipeline/items.go

@@ -73,10 +73,11 @@ func createBuildItems(ctx context.Context, store store.Store, build *model.Build
 	}
 	buildItems, err := b.Build()
 	if err != nil {
-		if _, err := shared.UpdateToStatusError(store, *build, err); err != nil {
+		build, uerr := shared.UpdateToStatusError(store, *build, err)
+		if uerr != nil {
 			log.Error().Err(err).Msgf("Error setting error status of build for %s#%d", repo.FullName, build.Number)
 		}
-		return nil, nil, err
+		return build, nil, err
 	}
 
 	build = shared.SetBuildStepsOnBuild(b.Curr, buildItems)

server/pipeline/restart.go

@@ -16,14 +16,17 @@ package pipeline
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
 	"github.com/rs/zerolog/log"
 
+	"github.com/woodpecker-ci/woodpecker/pipeline/frontend/yaml"
 	"github.com/woodpecker-ci/woodpecker/server"
 	"github.com/woodpecker-ci/woodpecker/server/model"
 	"github.com/woodpecker-ci/woodpecker/server/remote"
+	"github.com/woodpecker-ci/woodpecker/server/shared"
 	"github.com/woodpecker-ci/woodpecker/server/store"
 )
 
@@ -77,6 +80,13 @@ func Restart(ctx context.Context, store store.Store, lastBuild *model.Build, use
 		return nil, fmt.Errorf(msg)
 	}
 
+	if len(configs) == 0 {
+		newBuild, uerr := shared.UpdateToStatusError(store, *newBuild, errors.New("pipeline definition not found"))
+		if uerr != nil {
+			log.Debug().Err(uerr).Msg("failure to update pipeline status")
+		}
+		return newBuild, nil
+	}
 	if err := persistBuildConfigs(store, configs, newBuild.ID); err != nil {
 		msg := fmt.Sprintf("failure to persist build config for %s.", repo.FullName)
 		log.Error().Err(err).Msg(msg)
@@ -85,6 +95,9 @@ func Restart(ctx context.Context, store store.Store, lastBuild *model.Build, use
 
 	newBuild, buildItems, err := createBuildItems(ctx, store, newBuild, user, repo, pipelineFiles, envs)
 	if err != nil {
+		if errors.Is(err, &yaml.PipelineParseError{}) {
+			return newBuild, nil
+		}
 		msg := fmt.Sprintf("failure to createBuildItems for %s", repo.FullName)
 		log.Error().Err(err).Msg(msg)
 		return nil, fmt.Errorf(msg)

server/shared/procBuilder.go

@@ -107,14 +107,14 @@ func (b *ProcBuilder) Build() ([]*BuildItem, error) {
 			// parse yaml pipeline
 			parsed, err := yaml.ParseString(substituted)
 			if err != nil {
-				return nil, err
+				return nil, &yaml.PipelineParseError{Err: err}
 			}
 
 			// lint pipeline
 			if err := linter.New(
 				linter.WithTrusted(b.Repo.IsTrusted),
 			).Lint(parsed); err != nil {
-				return nil, err
+				return nil, &yaml.PipelineParseError{Err: err}
 			}
 
 			if !parsed.Branches.Match(b.Curr.Branch) && (b.Curr.Event != model.EventDeploy && b.Curr.Event != model.EventTag) {