improve error reporting on the RunSpec function (#1679)

diggerhq · Aug 27, 2024 · 9b72be0 · 9b72be0
1 parent 62d43de
commit 9b72be0
Showing 1 changed file with 46 additions and 19 deletions.
diff --git a/cli/pkg/spec/spec.go b/cli/pkg/spec/spec.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"github.com/diggerhq/digger/cli/pkg/digger"
 	"github.com/diggerhq/digger/cli/pkg/usage"
+	backend2 "github.com/diggerhq/digger/libs/backendapi"
 	comment_summary "github.com/diggerhq/digger/libs/comment_utils/summary"
 	"github.com/diggerhq/digger/libs/digger_config"
 	"github.com/diggerhq/digger/libs/scheduler"
@@ -14,6 +15,15 @@ import (
 	"time"
 )
 
+func reportError(spec spec.Spec, backendApi backend2.Api, message string, err error) {
+	log.Printf(message)
+	_, reportingError := backendApi.ReportProjectJobStatus(spec.VCS.RepoName, spec.Job.ProjectName, spec.JobId, "failed", time.Now(), nil, "", "")
+	if reportingError != nil {
+		usage.ReportErrorAndExit(spec.VCS.RepoOwner, fmt.Sprintf("Failed to run commands. %v", err), 5)
+	}
+	usage.ReportErrorAndExit(spec.VCS.Actor, message, 1)
+}
+
 func RunSpec(
 	spec spec.Spec,
 	vcsProvider spec.VCSProvider,
@@ -26,41 +36,50 @@ func RunSpec(
 	commentUpdaterProvider comment_summary.CommentUpdaterProvider,
 ) error {
 
+	backendApi, err := backedProvider.GetBackendApi(spec.Backend)
+	if err != nil {
+		log.Printf("could not get backend api: %v", err)
+		usage.ReportErrorAndExit(spec.VCS.Actor, fmt.Sprintf("could not get backend api: %v", err), 1)
+	}
+
 	job, err := jobProvider.GetJob(spec.Job)
 	if err != nil {
-		usage.ReportErrorAndExit(spec.VCS.Actor, fmt.Sprintf("could not get job: %v", err), 1)
+		message := fmt.Sprintf("could not get job: %v", err)
+		reportError(spec, backendApi, message, err)
 	}
 
 	lock, err := lockProvider.GetLock(spec.Lock)
 	if err != nil {
-		usage.ReportErrorAndExit(spec.VCS.Actor, fmt.Sprintf("could not get job: %v", err), 1)
-
+		message := fmt.Sprintf("could not get lock provider: %v", err)
+		reportError(spec, backendApi, message, err)
 	}
 
 	prService, err := vcsProvider.GetPrService(spec.VCS)
 	if err != nil {
-		usage.ReportErrorAndExit(spec.VCS.Actor, fmt.Sprintf("could not get prservice: %v", err), 1)
+		message := fmt.Sprintf("could not get pr service: %v", err)
+		reportError(spec, backendApi, message, err)
 	}
 
 	orgService, err := vcsProvider.GetOrgService(spec.VCS)
 	if err != nil {
-		usage.ReportErrorAndExit(spec.VCS.Actor, fmt.Sprintf("could not get orgservice: %v", err), 1)
-	}
-	reporter, err := reporterProvider.GetReporter(fmt.Sprintf("%v for %v", spec.Job.JobType, job.ProjectName), spec.Reporter, prService, *spec.Job.PullRequestNumber)
-	if err != nil {
-		usage.ReportErrorAndExit(spec.VCS.Actor, fmt.Sprintf("could not get reporter: %v", err), 1)
+		message := fmt.Sprintf("could not get org service: %v", err)
+		reportError(spec, backendApi, message, err)
 	}
 
-	backendApi, err := backedProvider.GetBackendApi(spec.Backend)
+	reporter, err := reporterProvider.GetReporter(fmt.Sprintf("%v for %v", spec.Job.JobType, job.ProjectName), spec.Reporter, prService, *spec.Job.PullRequestNumber)
 	if err != nil {
-		usage.ReportErrorAndExit(spec.VCS.Actor, fmt.Sprintf("could not get backend api: %v", err), 1)
+		message := fmt.Sprintf("could not get reporter: %v", err)
+		reportError(spec, backendApi, message, err)
 	}
 
 	policyChecker, err := policyProvider.GetPolicyProvider(spec.Policy, spec.Backend.BackendHostname, spec.Backend.BackendOrganisationName, spec.Backend.BackendJobToken)
 	if err != nil {
-		usage.ReportErrorAndExit(spec.VCS.Actor, fmt.Sprintf("could not get policy provider: %v", err), 1)
+		message := fmt.Sprintf("could not get policy provider: %v", err)
+		reportError(spec, backendApi, message, err)
 	}
 
+	// TODO: avoid calling GetChangedFilesHere, avoid loading digger config entirely
+	// also see below TODO to leverage variables provider and avoid passing it to commentUpdaterProvider
 	changedFiles, err := prService.GetChangedFiles(*spec.Job.PullRequestNumber)
 	if err != nil {
 		usage.ReportErrorAndExit(spec.VCS.Actor, fmt.Sprintf("could not get changed files: %v", err), 1)
@@ -71,16 +90,20 @@ func RunSpec(
 	}
 	log.Printf("Digger digger_config read successfully\n")
 
+	// TODO: instead of passing diggerConfig we simply pass the renderMode needed by comment updater provider
 	commentUpdater, err := commentUpdaterProvider.Get(*diggerConfig)
 	if err != nil {
-		usage.ReportErrorAndExit(spec.VCS.Actor, fmt.Sprintf("could not get comment updater: %v", err), 8)
+		message := fmt.Sprintf("could not get comment updater: %v", err)
+		reportError(spec, backendApi, message, err)
 	}
 
 	planStorage, err := PlanStorageProvider.GetPlanStorage(spec.VCS.RepoOwner, spec.VCS.RepoName, *spec.Job.PullRequestNumber)
 	if err != nil {
-		usage.ReportErrorAndExit(spec.VCS.Actor, fmt.Sprintf("could not get plan storage: %v", err), 8)
+		message := fmt.Sprintf("could not get planStorage: %v", err)
+		reportError(spec, backendApi, message, err)
 	}
 
+	// TODO: make this part purely based on variables providers
 	workflow := diggerConfig.Workflows[job.ProjectWorkflow]
 	stateEnvVars, commandEnvVars := digger_config.CollectTerraformEnvConfig(workflow.EnvVars)
 	job.StateEnvVars = lo.Assign(job.StateEnvVars, stateEnvVars)
@@ -91,29 +114,33 @@ func RunSpec(
 	fullRepoName := fmt.Sprintf("%v-%v", spec.VCS.RepoOwner, spec.VCS.RepoName)
 	_, err = backendApi.ReportProjectJobStatus(fullRepoName, spec.Job.ProjectName, spec.JobId, "started", time.Now(), nil, "", "")
 	if err != nil {
-		usage.ReportErrorAndExit(spec.VCS.Actor, fmt.Sprintf("Failed to report jobSpec status to backend. Exiting. %v", err), 4)
+		message := fmt.Sprintf("Failed to report jobSpec status to backend. Exiting. %v", err)
+		reportError(spec, backendApi, message, err)
 	}
 
 	commentId := spec.CommentId
 	if err != nil {
-		usage.ReportErrorAndExit(spec.VCS.Actor, fmt.Sprintf("failed to get comment ID: %v", err), 4)
+		message := fmt.Sprintf("failed to get comment ID: %v", err)
+		reportError(spec, backendApi, message, err)
 	}
 
 	currentDir, err := os.Getwd()
 	if err != nil {
-		usage.ReportErrorAndExit(spec.VCS.Actor, fmt.Sprintf("Failed to get current dir. %s", err), 4)
+		message := fmt.Sprintf("Failed to get current dir. %s", err)
+		reportError(spec, backendApi, message, err)
 	}
 
 	reportTerraformOutput := spec.Reporter.ReportTerraformOutput
 	allAppliesSuccess, _, err := digger.RunJobs(jobs, prService, orgService, lock, reporter, planStorage, policyChecker, commentUpdater, backendApi, spec.JobId, true, reportTerraformOutput, commentId, currentDir)
 	if !allAppliesSuccess || err != nil {
 		serializedBatch, reportingError := backendApi.ReportProjectJobStatus(spec.VCS.RepoName, spec.Job.ProjectName, spec.JobId, "failed", time.Now(), nil, "", "")
 		if reportingError != nil {
-			usage.ReportErrorAndExit(spec.VCS.RepoOwner, fmt.Sprintf("Failed run commands. %s", err), 5)
+			message := fmt.Sprintf("Failed run commands. %s", err)
+			reportError(spec, backendApi, message, err)
 		}
 		commentUpdater.UpdateComment(serializedBatch.Jobs, serializedBatch.PrNumber, prService, commentId)
 		digger.UpdateAggregateStatus(serializedBatch, prService)
-		usage.ReportErrorAndExit(spec.VCS.RepoOwner, fmt.Sprintf("Failed to run commands. %s", err), 5)
+		reportError(spec, backendApi, fmt.Sprintf("Failed to run commands. %s", err), err)
 	}
 	usage.ReportErrorAndExit(spec.VCS.RepoOwner, "Digger finished successfully", 0)