Avoid panic caused by broken payload when creating commit status (#23216)

When creating commit status for Actons jobs, a payload with nil
`HeadCommit` will cause panic.

Reported at:
https://gitea.com/gitea/act_runner/issues/28#issuecomment-732166

Although the `HeadCommit` probably can not be nil after #23215,
`CreateCommitStatus` should protect itself, to avoid being broken in the
future.

In addition, it's enough to print error log instead of returning err
when `CreateCommitStatus` failed.

---------

Co-authored-by: delvh <dev.lh@web.de>

3 files changed, 29 insertions, 6 deletions

diff --git a/services/actions/clear_tasks.go b/services/actions/clear_tasks.go
index 583e588de..6f8e95218 100644
--- a/services/actions/clear_tasks.go
+++ b/services/actions/clear_tasks.go
@@ -43,6 +43,7 @@ func stopTasks(ctx context.Context, opts actions_model.FindTaskOptions) error {
 		return fmt.Errorf("find tasks: %w", err)
 	}
 
+	jobs := make([]*actions_model.ActionRunJob, 0, len(tasks))
 	for _, task := range tasks {
 		if err := db.WithTx(ctx, func(ctx context.Context) error {
 			if err := actions_model.StopTask(ctx, task.ID, actions_model.StatusFailure); err != nil {
@@ -51,7 +52,8 @@ func stopTasks(ctx context.Context, opts actions_model.FindTaskOptions) error {
 			if err := task.LoadJob(ctx); err != nil {
 				return err
 			}
-			return CreateCommitStatus(ctx, task.Job)
+			jobs = append(jobs, task.Job)
+			return nil
 		}); err != nil {
 			log.Warn("Cannot stop task %v: %v", task.ID, err)
 			// go on
@@ -61,6 +63,14 @@ func stopTasks(ctx context.Context, opts actions_model.FindTaskOptions) error {
 			remove()
 		}
 	}
+
+	for _, job := range jobs {
+		if err := CreateCommitStatus(ctx, job); err != nil {
+			log.Error("Update commit status for job %v failed: %v", job.ID, err)
+			// go on
+		}
+	}
+
 	return nil
 }
 
@@ -80,14 +90,16 @@ func CancelAbandonedJobs(ctx context.Context) error {
 		job.Status = actions_model.StatusCancelled
 		job.Stopped = now
 		if err := db.WithTx(ctx, func(ctx context.Context) error {
-			if _, err := actions_model.UpdateRunJob(ctx, job, nil, "status", "stopped"); err != nil {
-				return err
-			}
-			return CreateCommitStatus(ctx, job)
+			_, err := actions_model.UpdateRunJob(ctx, job, nil, "status", "stopped")
+			return err
 		}); err != nil {
 			log.Warn("cancel abandoned job %v: %v", job.ID, err)
 			// go on
 		}
+		if err := CreateCommitStatus(ctx, job); err != nil {
+			log.Error("Update commit status for job %v failed: %v", job.ID, err)
+			// go on
+		}
 	}
 
 	return nil
diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go
index efb5ec6d4..4f3134935 100644
--- a/services/actions/commit_status.go
+++ b/services/actions/commit_status.go
@@ -30,6 +30,16 @@ func CreateCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er
 		return fmt.Errorf("GetPushEventPayload: %w", err)
 	}
 
+	// Since the payload comes from json data, we should check if it's broken, or it will cause panic
+	switch {
+	case payload.Repo == nil:
+		return fmt.Errorf("repo is missing in event payload")
+	case payload.Pusher == nil:
+		return fmt.Errorf("pusher is missing in event payload")
+	case payload.HeadCommit == nil:
+		return fmt.Errorf("head commit is missing in event payload")
+	}
+
 	creator, err := user_model.GetUserByID(ctx, payload.Pusher.ID)
 	if err != nil {
 		return fmt.Errorf("GetUserByID: %w", err)
diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go
index ef63b8cf9..574a37e9a 100644
--- a/services/actions/notifier_helper.go
+++ b/services/actions/notifier_helper.go
@@ -187,7 +187,8 @@ func notify(ctx context.Context, input *notifyInput) error {
 		} else {
 			for _, job := range jobs {
 				if err := CreateCommitStatus(ctx, job); err != nil {
-					log.Error("CreateCommitStatus: %v", err)
+					log.Error("Update commit status for job %v failed: %v", job.ID, err)
+					// go on
 				}
 			}
 		}