diff --git a/server/api/build.go b/server/api/build.go index 997cf507b..1d43ca593 100644 --- a/server/api/build.go +++ b/server/api/build.go @@ -74,7 +74,10 @@ func GetBuild(c *gin.Context) { } files, _ := _store.FileList(build) procs, _ := _store.ProcList(build) - build.Procs = model.Tree(procs) + if build.Procs, err = model.Tree(procs); err != nil { + _ = c.AbortWithError(http.StatusInternalServerError, err) + return + } build.Files = files c.JSON(http.StatusOK, build) @@ -91,8 +94,15 @@ func GetBuildLast(c *gin.Context) { return } - procs, _ := _store.ProcList(build) - build.Procs = model.Tree(procs) + procs, err := _store.ProcList(build) + if err != nil { + _ = c.AbortWithError(http.StatusInternalServerError, err) + return + } + if build.Procs, err = model.Tree(procs); err != nil { + _ = c.AbortWithError(http.StatusInternalServerError, err) + return + } c.JSON(http.StatusOK, build) } @@ -250,7 +260,10 @@ func DeleteBuild(c *gin.Context) { _ = c.AbortWithError(404, err) return } - killedBuild.Procs = model.Tree(procs) + if killedBuild.Procs, err = model.Tree(procs); err != nil { + _ = c.AbortWithError(http.StatusInternalServerError, err) + return + } if err := publishToTopic(c, killedBuild, repo, model.Canceled); err != nil { log.Error().Err(err).Msg("publishToTopic") } diff --git a/server/api/hook.go b/server/api/hook.go index e82a4c20e..3b258ed96 100644 --- a/server/api/hook.go +++ b/server/api/hook.go @@ -371,7 +371,7 @@ func findOrPersistPipelineConfig(repo *model.Repo, build *model.Build, remoteYam } // publishes message to UI clients -func publishToTopic(c *gin.Context, build *model.Build, repo *model.Repo, event model.EventType) error { +func publishToTopic(c *gin.Context, build *model.Build, repo *model.Repo, event model.EventType) (err error) { message := pubsub.Message{ Labels: map[string]string{ "repo": repo.FullName, @@ -379,7 +379,10 @@ func publishToTopic(c *gin.Context, build *model.Build, repo *model.Repo, event }, } buildCopy := *build - buildCopy.Procs = model.Tree(buildCopy.Procs) + if buildCopy.Procs, err = model.Tree(buildCopy.Procs); err != nil { + return err + } + message.Data, _ = json.Marshal(model.Event{ Type: model.Enqueued, Repo: *repo, diff --git a/server/grpc/rpc.go b/server/grpc/rpc.go index 1782c80f0..530b42f22 100644 --- a/server/grpc/rpc.go +++ b/server/grpc/rpc.go @@ -137,11 +137,16 @@ func (s *RPC) Update(c context.Context, id string, state rpc.State) error { } if _, err = shared.UpdateProcStatus(s.store, *proc, state, build.Started); err != nil { - log.Error().Msgf("error: rpc.update: cannot update proc: %s", err) + log.Error().Err(err).Msg("rpc.update: cannot update proc") } - build.Procs, _ = s.store.ProcList(build) - build.Procs = model.Tree(build.Procs) + if build.Procs, err = s.store.ProcList(build); err != nil { + log.Error().Err(err).Msg("can not get proc list from store") + } + if build.Procs, err = model.Tree(build.Procs); err != nil { + log.Error().Err(err).Msg("can not build tree from proc list") + return err + } message := pubsub.Message{ Labels: map[string]string{ "repo": repo.FullName, @@ -330,12 +335,15 @@ func (s *RPC) Done(c context.Context, id string, state rpc.State) error { log.Error().Msgf("error: done: cannot ack proc_id %d: %s", procID, err) } - procs, _ := s.store.ProcList(build) + procs, err := s.store.ProcList(build) + if err != nil { + return err + } s.completeChildrenIfParentCompleted(procs, proc) if !isThereRunningStage(procs) { if build, err = shared.UpdateStatusToDone(s.store, *build, buildStatus(procs), proc.Stopped); err != nil { - log.Error().Msgf("error: done: cannot update build_id %d final state: %s", build.ID, err) + log.Error().Err(err).Msgf("error: done: cannot update build_id %d final state", build.ID) } if !isMultiPipeline(procs) { @@ -348,10 +356,12 @@ func (s *RPC) Done(c context.Context, id string, state rpc.State) error { } if err := s.logger.Close(c, id); err != nil { - log.Error().Msgf("error: done: cannot close build_id %d logger: %s", proc.ID, err) + log.Error().Err(err).Msgf("done: cannot close build_id %d logger", proc.ID) } - s.notify(c, repo, build, procs) + if err := s.notify(c, repo, build, procs); err != nil { + return err + } if build.Status == model.StatusSuccess || build.Status == model.StatusFailure { s.buildCount.WithLabelValues(repo.FullName, build.Branch, string(build.Status), "total").Inc() @@ -440,8 +450,10 @@ func (s *RPC) updateRemoteStatus(ctx context.Context, repo *model.Repo, build *m } } -func (s *RPC) notify(c context.Context, repo *model.Repo, build *model.Build, procs []*model.Proc) { - build.Procs = model.Tree(procs) +func (s *RPC) notify(c context.Context, repo *model.Repo, build *model.Build, procs []*model.Proc) (err error) { + if build.Procs, err = model.Tree(procs); err != nil { + return err + } message := pubsub.Message{ Labels: map[string]string{ "repo": repo.FullName, @@ -455,6 +467,7 @@ func (s *RPC) notify(c context.Context, repo *model.Repo, build *model.Build, pr if err := s.pubsub.Publish(c, "topic/events", message); err != nil { log.Error().Err(err).Msgf("grpc could not notify event: '%v'", message) } + return nil } func createFilterFunc(filter rpc.Filter) (queue.Filter, error) { diff --git a/server/model/proc.go b/server/model/proc.go index 89f3a7332..c3c7a2a71 100644 --- a/server/model/proc.go +++ b/server/model/proc.go @@ -64,17 +64,20 @@ func (p *Proc) Failing() bool { } // Tree creates a process tree from a flat process list. -func Tree(procs []*Proc) []*Proc { +func Tree(procs []*Proc) ([]*Proc, error) { var nodes []*Proc for _, proc := range procs { if proc.PPID == 0 { nodes = append(nodes, proc) } else { - parent, _ := findNode(nodes, proc.PPID) + parent, err := findNode(nodes, proc.PPID) + if err != nil { + return nil, err + } parent.Children = append(parent.Children, proc) } } - return nodes + return nodes, nil } func findNode(nodes []*Proc, pid int) (*Proc, error) {