fix(webhook): avoid panics (#671)

api/build.go

@@ -1455,21 +1455,36 @@ func getPRNumberFromBuild(b *library.Build) (int, error) {
 // planBuild is a helper function to plan the build for
 // execution. This creates all resources, like steps
 // and services, for the build in the configured backend.
+// TODO:
+// - return build and error
 func planBuild(database database.Service, p *pipeline.Build, b *library.Build, r *library.Repo) error {
 	// update fields in build object
 	b.SetCreated(time.Now().UTC().Unix())
 
 	// send API call to create the build
+	// TODO: return created build and error instead of just error
 	err := database.CreateBuild(b)
 	if err != nil {
 		// clean up the objects from the pipeline in the database
+		// TODO:
+		// - return build in CreateBuild
+		// - even if it was created, we need to get the new build id
+		//   otherwise it will be 0, which attempts to INSERT instead
+		//   of UPDATE-ing the existing build - which results in
+		//   a constraint error (repo_id, number)
+		// - do we want to update the build or just delete it?
 		cleanBuild(database, b, nil, nil)
 
 		return fmt.Errorf("unable to create new build for %s: %w", r.GetFullName(), err)
 	}
 
 	// send API call to capture the created build
-	b, _ = database.GetBuild(b.GetNumber(), r)
+	// TODO: this can be dropped once we return
+	// the created build above
+	b, err = database.GetBuild(b.GetNumber(), r)
+	if err != nil {
+		return fmt.Errorf("unable to get new build for %s: %w", r.GetFullName(), err)
+	}
 
 	// plan all services for the build
 	services, err := planServices(database, p, b)

api/webhook.go

@@ -402,14 +402,6 @@ func PostWebhook(c *gin.Context) {
 			if err != nil {
 				retErr := fmt.Errorf("%s: unable to get pipeline configuration for %s: %w", baseErr, r.GetFullName(), err)
 
-				// check if the retry limit has been exceeded
-				if i < retryLimit {
-					logrus.WithError(retErr).Warning("retrying")
-
-					// continue to the next iteration of the loop
-					continue
-				}
-
 				util.HandleError(c, http.StatusNotFound, retErr)
 
 				h.SetStatus(constants.StatusFailure)
@@ -427,8 +419,8 @@ func PostWebhook(c *gin.Context) {
 			retErr := fmt.Errorf("%s: unable to get repo %s: %w", baseErr, r.GetFullName(), err)
 
 			// check if the retry limit has been exceeded
-			if i < retryLimit {
-				logrus.WithError(retErr).Warning("retrying")
+			if i < retryLimit-1 {
+				logrus.WithError(retErr).Warningf("retrying #%d", i+1)
 
 				// continue to the next iteration of the loop
 				continue
@@ -499,6 +491,7 @@ func PostWebhook(c *gin.Context) {
 
 			return
 		}
+
 		// reset the pipeline type for the repo
 		//
 		// The pipeline type for a repo can change at any time which can break compiling
@@ -537,8 +530,8 @@ func PostWebhook(c *gin.Context) {
 				retErr := fmt.Errorf("%s: failed to create pipeline for %s: %w", baseErr, r.GetFullName(), err)
 
 				// check if the retry limit has been exceeded
-				if i < retryLimit {
-					logrus.WithError(retErr).Warning("retrying")
+				if i < retryLimit-1 {
+					logrus.WithError(retErr).Warningf("retrying #%d", i+1)
 
 					// continue to the next iteration of the loop
 					continue
@@ -569,13 +562,19 @@ func PostWebhook(c *gin.Context) {
 		b.SetPipelineID(pipeline.GetID())
 
 		// create the objects from the pipeline in the database
+		// TODO:
+		// - if a build gets created and something else fails midway,
+		//   the next loop will attempt to create the same build,
+		//   using the same Number and thus create a constraint
+		//   conflict; consider deleting the partially created
+		//   build object in the database
 		err = planBuild(database.FromContext(c), p, b, r)
 		if err != nil {
 			retErr := fmt.Errorf("%s: %w", baseErr, err)
 
 			// check if the retry limit has been exceeded
-			if i < retryLimit {
-				logrus.WithError(retErr).Warning("retrying")
+			if i < retryLimit-1 {
+				logrus.WithError(retErr).Warningf("retrying #%d", i+1)
 
 				// reset fields set by cleanBuild for retry
 				b.SetError("")
@@ -596,7 +595,7 @@ func PostWebhook(c *gin.Context) {
 
 		// break the loop because everything was successful
 		break
-	}
+	} // end of retry loop
 
 	// send API call to update repo for ensuring counter is incremented
 	err = database.FromContext(c).UpdateRepo(r)
@@ -610,6 +609,28 @@ func PostWebhook(c *gin.Context) {
 		return
 	}
 
+	// return error if pipeline didn't get populated
+	if p == nil {
+		retErr := fmt.Errorf("%s: failed to set pipeline for %s: %w", baseErr, r.GetFullName(), err)
+		util.HandleError(c, http.StatusBadRequest, retErr)
+
+		h.SetStatus(constants.StatusFailure)
+		h.SetError(retErr.Error())
+
+		return
+	}
+
+	// return error if build didn't get populated
+	if b == nil {
+		retErr := fmt.Errorf("%s: failed to set build for %s: %w", baseErr, r.GetFullName(), err)
+		util.HandleError(c, http.StatusBadRequest, retErr)
+
+		h.SetStatus(constants.StatusFailure)
+		h.SetError(retErr.Error())
+
+		return
+	}
+
 	// send API call to capture the triggered build
 	b, err = database.FromContext(c).GetBuild(b.GetNumber(), r)
 	if err != nil {
@@ -618,6 +639,8 @@ func PostWebhook(c *gin.Context) {
 
 		h.SetStatus(constants.StatusFailure)
 		h.SetError(retErr.Error())
+
+		return
 	}
 
 	// set the BuildID field