Dont panic, report error back (#582)

dont panic! return error back

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")
 		}

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,

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) {

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) {