aptly placed and phrased error messages

api/server/handlers/environment/create.go

@@ -1,6 +1,7 @@
 package environment
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"net/http"
@@ -87,7 +88,7 @@ func (c *CreateEnvironmentHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 
 	// create incoming webhook
 	hook, _, err := client.Repositories.CreateHook(
-		r.Context(), owner, name, &github.Hook{
+		context.Background(), owner, name, &github.Hook{
 			Config: map[string]interface{}{
 				"url":          webhookURL,
 				"content_type": "json",
@@ -98,10 +99,9 @@ func (c *CreateEnvironmentHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 		},
 	)
 
-	if err != nil && !strings.Contains(err.Error(), "already exists on this repository") {
-		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(
-			fmt.Errorf("error trying to create a new github repository webhook: %w", err), http.StatusConflict),
-		)
+	if err != nil && !strings.Contains(err.Error(), "already exists") {
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(fmt.Errorf("%v: %w", errGithubAPI, err),
+			http.StatusConflict))
 		return
 	}
 
@@ -110,6 +110,14 @@ func (c *CreateEnvironmentHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 	env, err = c.Repo().Environment().CreateEnvironment(env)
 
 	if err != nil {
+		_, deleteErr := client.Repositories.DeleteHook(context.Background(), owner, name, hook.GetID())
+
+		if deleteErr != nil {
+			c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(fmt.Errorf("%v: %w", errGithubAPI, deleteErr),
+				http.StatusConflict, "error creating environment"))
+			return
+		}
+
 		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
 		return
 	}
@@ -118,14 +126,44 @@ func (c *CreateEnvironmentHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 	jwt, err := token.GetTokenForAPI(user.ID, project.ID)
 
 	if err != nil {
-		c.deleteEnvAndReportError(w, r, env, err)
+		_, deleteErr := client.Repositories.DeleteHook(context.Background(), owner, name, hook.GetID())
+
+		if deleteErr != nil {
+			c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(fmt.Errorf("%v: %w", errGithubAPI, deleteErr),
+				http.StatusConflict, "error getting token for API while creating environment"))
+			return
+		}
+
+		_, deleteErr = c.Repo().Environment().DeleteEnvironment(env)
+
+		if deleteErr != nil {
+			c.HandleAPIError(w, r, apierrors.NewErrInternal(deleteErr))
+			return
+		}
+
+		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
 		return
 	}
 
 	encoded, err := jwt.EncodeToken(c.Config().TokenConf)
 
 	if err != nil {
-		c.deleteEnvAndReportError(w, r, env, err)
+		_, deleteErr := client.Repositories.DeleteHook(context.Background(), owner, name, hook.GetID())
+
+		if deleteErr != nil {
+			c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(fmt.Errorf("%v: %w", errGithubAPI, deleteErr),
+				http.StatusConflict, "error encoding token while creating environment"))
+			return
+		}
+
+		_, deleteErr = c.Repo().Environment().DeleteEnvironment(env)
+
+		if deleteErr != nil {
+			c.HandleAPIError(w, r, apierrors.NewErrInternal(deleteErr))
+			return
+		}
+
+		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
 		return
 	}
 
@@ -159,19 +197,6 @@ func (c *CreateEnvironmentHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 	c.WriteResult(w, r, env.ToEnvironmentType())
 }
 
-func (c *CreateEnvironmentHandler) deleteEnvAndReportError(
-	w http.ResponseWriter, r *http.Request, env *models.Environment, err error,
-) {
-	_, delErr := c.Repo().Environment().DeleteEnvironment(env)
-
-	if delErr != nil {
-		c.HandleAPIError(w, r, apierrors.NewErrInternal(delErr))
-		return
-	}
-
-	c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
-}
-
 func getGithubClientFromEnvironment(config *config.Config, env *models.Environment) (*github.Client, error) {
 	// get the github app client
 	ghAppId, err := strconv.Atoi(config.ServerConf.GithubAppID)

api/server/handlers/environment/delete.go

@@ -17,6 +17,7 @@ import (
 	"github.com/porter-dev/porter/internal/integrations/ci/actions"
 	"github.com/porter-dev/porter/internal/models"
 	"github.com/porter-dev/porter/internal/models/integrations"
+	"gorm.io/gorm"
 )
 
 type DeleteEnvironmentHandler struct {
@@ -50,14 +51,11 @@ func (c *DeleteEnvironmentHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 	env, err := c.Repo().Environment().ReadEnvironment(project.ID, cluster.ID, uint(ga.InstallationID), owner, name)
 
 	if err != nil {
-		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
-		return
-	}
-
-	// delete Github actions files from the repo
-	client, err := getGithubClientFromEnvironment(c.Config(), env)
+		if errors.Is(err, gorm.ErrRecordNotFound) {
+			c.HandleAPIError(w, r, apierrors.NewErrNotFound(fmt.Errorf("environment not found in cluster and project")))
+			return
+		}
 
-	if err != nil {
 		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
 		return
 	}
@@ -92,6 +90,13 @@ func (c *DeleteEnvironmentHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 		return
 	}
 
+	client, err := getGithubClientFromEnvironment(c.Config(), env)
+
+	if err != nil {
+		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
+		return
+	}
+
 	// FIXME: ignore the return status codes for now, should be fixed when we start returning all non-fatal errors
 	if ghWebhookID != 0 {
 		client.Repositories.DeleteHook(context.Background(), owner, name, ghWebhookID)
@@ -129,7 +134,7 @@ func (c *DeleteEnvironmentHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 	if err != nil {
 		if errors.Is(err, actions.ErrProtectedBranch) {
 			c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(
-				fmt.Errorf("We were unable to delete the Porter Preview Environment workflow files for this "+
+				fmt.Errorf("we were unable to delete the Porter Preview Environment workflow files for this "+
 					"repository as the default branch is protected. Please manually delete them."), http.StatusConflict,
 			))
 			return

api/server/handlers/environment/delete_deployment.go

@@ -50,6 +50,11 @@ func (c *DeleteDeploymentHandler) ServeHTTP(w http.ResponseWriter, r *http.Reque
 	depl, err := c.Repo().Environment().ReadDeploymentByID(project.ID, cluster.ID, deplID)
 
 	if err != nil {
+		if errors.Is(err, gorm.ErrRecordNotFound) {
+			c.HandleAPIError(w, r, apierrors.NewErrNotFound(fmt.Errorf("deployment id not found in cluster and project")))
+			return
+		}
+
 		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
 		return
 	}
@@ -85,6 +90,16 @@ func (c *DeleteDeploymentHandler) ServeHTTP(w http.ResponseWriter, r *http.Reque
 		return
 	}
 
+	depl.Status = types.DeploymentStatusInactive
+
+	// update the deployment to mark it inactive
+	depl, err = c.Repo().Environment().UpdateDeployment(depl)
+
+	if err != nil {
+		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
+		return
+	}
+
 	client, err := getGithubClientFromEnvironment(c.Config(), env)
 
 	if err != nil {
@@ -105,20 +120,12 @@ func (c *DeleteDeploymentHandler) ServeHTTP(w http.ResponseWriter, r *http.Reque
 		)
 
 		if err != nil {
-			c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
+			c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(
+				fmt.Errorf("%v: %w", errGithubAPI, err), http.StatusConflict,
+			))
 			return
 		}
 	}
 
-	depl.Status = types.DeploymentStatusInactive
-
-	// update the deployment to mark it inactive
-	depl, err = c.Repo().Environment().UpdateDeployment(depl)
-
-	if err != nil {
-		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
-		return
-	}
-
 	c.WriteResult(w, r, depl.ToDeploymentType())
 }

api/server/handlers/environment/enable_pull_request.go

@@ -1,6 +1,7 @@
 package environment
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
 	"strconv"
@@ -14,6 +15,7 @@ import (
 	"github.com/porter-dev/porter/api/server/shared/config"
 	"github.com/porter-dev/porter/api/types"
 	"github.com/porter-dev/porter/internal/models"
+	"gorm.io/gorm"
 )
 
 type EnablePullRequestHandler struct {
@@ -44,6 +46,11 @@ func (c *EnablePullRequestHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 	env, err := c.Repo().Environment().ReadEnvironmentByOwnerRepoName(project.ID, cluster.ID, request.RepoOwner, request.RepoName)
 
 	if err != nil {
+		if errors.Is(err, gorm.ErrRecordNotFound) {
+			c.HandleAPIError(w, r, apierrors.NewErrNotFound(fmt.Errorf("environment not found in cluster and project")))
+			return
+		}
+
 		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
 		return
 	}
@@ -59,12 +66,13 @@ func (c *EnablePullRequestHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 	pr, _, err := client.PullRequests.Get(r.Context(), env.GitRepoOwner, env.GitRepoName, int(request.Number))
 
 	if err != nil {
-		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(fmt.Errorf("%v: %w", errGithubAPI, err),
+			http.StatusConflict))
 		return
 	}
 
 	if pr.GetState() == "closed" {
-		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(fmt.Errorf("Github PR has been closed"),
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(fmt.Errorf("cannot enable deployment for closed PR"),
 			http.StatusConflict))
 		return
 	}
@@ -86,17 +94,17 @@ func (c *EnablePullRequestHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 		if ghResp.StatusCode == 404 {
 			c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(
 				fmt.Errorf(
-					"Please make sure the preview environment workflow files are present in PR branch %s and are up to"+
+					"please make sure the preview environment workflow files are present in PR branch %s and are up to"+
 						" date with the default branch", request.BranchFrom,
-				), 404),
+				), http.StatusConflict),
 			)
 			return
 		} else if ghResp.StatusCode == 422 {
 			c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(
 				fmt.Errorf(
-					"Please make sure the workflow files in PR branch %s are up to date with the default branch",
+					"please make sure the workflow files in PR branch %s are up to date with the default branch",
 					request.BranchFrom,
-				), 422),
+				), http.StatusConflict),
 			)
 			return
 		}
@@ -141,5 +149,4 @@ func (c *EnablePullRequestHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ
 		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
 		return
 	}
-
 }

api/server/handlers/environment/finalize_deployment_with_errors.go

@@ -61,7 +61,7 @@ func (c *FinalizeDeploymentWithErrorsHandler) ServeHTTP(w http.ResponseWriter, r
 
 	if err != nil {
 		if errors.Is(err, gorm.ErrRecordNotFound) {
-			c.HandleAPIError(w, r, apierrors.NewErrNotFound(fmt.Errorf("no environment found")))
+			c.HandleAPIError(w, r, apierrors.NewErrNotFound(fmt.Errorf("environment not found in cluster and project")))
 			return
 		}
 
@@ -85,9 +85,7 @@ func (c *FinalizeDeploymentWithErrorsHandler) ServeHTTP(w http.ResponseWriter, r
 	client, err := getGithubClientFromEnvironment(c.Config(), env)
 
 	if err != nil {
-		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(
-			fmt.Errorf("unable to get github client: %w", err), http.StatusConflict,
-		))
+		c.HandleAPIError(w, r, apierrors.NewErrInternal(err))
 		return
 	}
 
@@ -95,15 +93,12 @@ func (c *FinalizeDeploymentWithErrorsHandler) ServeHTTP(w http.ResponseWriter, r
 	prClosed, err := isGithubPRClosed(client, owner, name, int(depl.PullRequestID))
 
 	if err != nil {
-		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(
-			fmt.Errorf("error fetching details of github PR for deployment ID: %d. Error: %w",
-				depl.ID, err), http.StatusConflict,
-		))
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(err, http.StatusConflict))
 		return
 	}
 
 	if prClosed {
-		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(fmt.Errorf("Github PR has been closed"),
+		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(fmt.Errorf("github PR has been closed"),
 			http.StatusConflict))
 		return
 	}