Merge pull request #2877 from ActiveState/mitchell/dx-2232

Initial implementation of user-facing errors for `state install` and `state uninstall`.

internal/locale/locales/en-us.yaml

@@ -1150,6 +1150,8 @@ bundle_version_updated:
   other: "Bundle updated: [NOTICE]{{.V0}}@{{.V1}}[/RESET]"
 err_package_removed:
   other: Failed to remove package
+err_remove_requirement_not_found:
+  other: Could not remove requirement '[ACTIONABLE]{{.V0}}[/RESET]', because it does not exist.
 err_bundle_removed:
   other: Failed to remove bundle
 err_packages_removed:
@@ -1747,6 +1749,8 @@ operation_success_local:
     Run [ACTIONABLE]state push[/RESET] to save changes to the platform.
 buildplan_err:
   other: "Could not plan build, platform responded with:\n{{.V0}}\n{{.V1}}"
+err_buildplanner_head_on_branch_moved:
+  other: The branch you're trying to update has changed remotely, please run '[ACTIONABLE]state pull[/RESET]'.
 transient_solver_tip:
   other: You may want to retry this command if the failure was related to a resourcing or networking issue.
 warning_git_project_mismatch:

internal/runners/packages/install.go

@@ -37,7 +37,8 @@ func NewInstall(prime primeable) *Install {
 }
 
 // Run executes the install behavior.
-func (a *Install) Run(params InstallRunParams, nsType model.NamespaceType) error {
+func (a *Install) Run(params InstallRunParams, nsType model.NamespaceType) (rerr error) {
+	defer rationalizeError(a.prime.Auth(), &rerr)
 	logging.Debug("ExecuteInstall")
 	return requirements.NewRequirementOperation(a.prime).ExecuteRequirementOperation(
 		params.Package.Name(),

internal/runners/packages/rationalize.go

@@ -0,0 +1,68 @@
+package packages
+
+import (
+	"errors"
+
+	"github.com/ActiveState/cli/internal/errs"
+	"github.com/ActiveState/cli/internal/locale"
+	"github.com/ActiveState/cli/internal/runbits/rationalize"
+	bpModel "github.com/ActiveState/cli/pkg/platform/api/buildplanner/model"
+	"github.com/ActiveState/cli/pkg/platform/authentication"
+	"github.com/ActiveState/cli/pkg/platform/runtime/buildexpression"
+)
+
+func rationalizeError(auth *authentication.Auth, err *error) {
+	var commitError *bpModel.CommitError
+	var requirementNotFoundErr *buildexpression.RequirementNotFoundError
+
+	switch {
+	case err == nil:
+		return
+
+	// No activestate.yaml.
+	case errors.Is(*err, rationalize.ErrNoProject):
+		*err = errs.WrapUserFacing(*err,
+			locale.T("err_no_project"),
+			errs.SetInput(),
+		)
+
+	// Error staging a commit during install.
+	case errors.As(*err, &commitError):
+		switch commitError.Type {
+		case bpModel.NotFoundErrorType:
+			*err = errs.WrapUserFacing(*err,
+				locale.Tl("err_packages_not_found", "Could not make runtime changes because your project was not found."),
+				errs.SetInput(),
+				errs.SetTips(locale.T("tip_private_project_auth")),
+			)
+		case bpModel.ForbiddenErrorType:
+			*err = errs.WrapUserFacing(*err,
+				locale.Tl("err_packages_forbidden", "Could not make runtime changes because you do not have permission to do so."),
+				errs.SetInput(),
+				errs.SetTips(locale.T("tip_private_project_auth")),
+			)
+		case bpModel.HeadOnBranchMovedErrorType:
+			*err = errs.WrapUserFacing(*err,
+				locale.T("err_buildplanner_head_on_branch_moved"),
+				errs.SetInput(),
+			)
+		case bpModel.NoChangeSinceLastCommitErrorType:
+			*err = errs.WrapUserFacing(*err,
+				locale.Tl("err_packages_exists", "That package is already installed."),
+				errs.SetInput(),
+			)
+		default:
+			*err = errs.WrapUserFacing(*err,
+				locale.Tl("err_packages_buildplanner_error", "Could not make runtime changes due to the following error: {{.V0}}", commitError.Message),
+				errs.SetInput(),
+			)
+		}
+
+	// Requirement not found for uninstall.
+	case errors.As(*err, &requirementNotFoundErr):
+		*err = errs.WrapUserFacing(*err,
+			locale.Tr("err_remove_requirement_not_found", requirementNotFoundErr.Name),
+			errs.SetInput(),
+		)
+	}
+}

internal/runners/packages/uninstall.go

@@ -1,8 +1,8 @@
 package packages
 
 import (
-	"github.com/ActiveState/cli/internal/locale"
 	"github.com/ActiveState/cli/internal/logging"
+	"github.com/ActiveState/cli/internal/runbits/rationalize"
 	"github.com/ActiveState/cli/internal/runbits/requirements"
 	bpModel "github.com/ActiveState/cli/pkg/platform/api/buildplanner/model"
 	"github.com/ActiveState/cli/pkg/platform/model"
@@ -24,10 +24,11 @@ func NewUninstall(prime primeable) *Uninstall {
 }
 
 // Run executes the uninstall behavior.
-func (u *Uninstall) Run(params UninstallRunParams, nsType model.NamespaceType) error {
+func (u *Uninstall) Run(params UninstallRunParams, nsType model.NamespaceType) (rerr error) {
+	defer rationalizeError(u.prime.Auth(), &rerr)
 	logging.Debug("ExecuteUninstall")
 	if u.prime.Project() == nil {
-		return locale.NewInputError("err_no_project")
+		return rationalize.ErrNoProject
 	}
 
 	return requirements.NewRequirementOperation(u.prime).ExecuteRequirementOperation(

pkg/platform/api/buildplanner/model/buildplan.go

@@ -302,22 +302,43 @@ func IsErrorResponse(errorType string) bool {
 		errorType == ComitHasNoParentErrorType
 }
 
+type CommitError struct {
+	Type                   string
+	Message                string
+	*locale.LocalizedError // for legacy, non-user-facing error usages
+}
+
 func ProcessCommitError(commit *Commit, fallbackMessage string) error {
 	if commit.Error == nil {
 		return errs.New(fallbackMessage)
 	}
 
 	switch commit.Type {
 	case NotFoundErrorType:
-		return locale.NewInputError("err_buildplanner_commit_not_found", "Could not find commit, received message: {{.V0}}", commit.Message)
+		return &CommitError{
+			commit.Type, commit.Message,
+			locale.NewInputError("err_buildplanner_commit_not_found", "Could not find commit, received message: {{.V0}}", commit.Message),
+		}
 	case ParseErrorType:
-		return locale.NewInputError("err_buildplanner_parse_error", "The platform failed to parse the build expression, received message: {{.V0}}. Path: {{.V1}}", commit.Message, commit.ParseError.Path)
+		return &CommitError{
+			commit.Type, commit.Message,
+			locale.NewInputError("err_buildplanner_parse_error", "The platform failed to parse the build expression, received message: {{.V0}}. Path: {{.V1}}", commit.Message, commit.ParseError.Path),
+		}
 	case ForbiddenErrorType:
-		return locale.NewInputError("err_buildplanner_forbidden", "Operation forbidden: {{.V0}}, received message: {{.V1}}", commit.Operation, commit.Message)
+		return &CommitError{
+			commit.Type, commit.Message,
+			locale.NewInputError("err_buildplanner_forbidden", "Operation forbidden: {{.V0}}, received message: {{.V1}}", commit.Operation, commit.Message),
+		}
 	case HeadOnBranchMovedErrorType:
-		return errs.Wrap(locale.NewInputError("err_buildplanner_head_on_branch_moved", "The branch you're trying to update has changed remotely, please run '[ACTIONABLE]state pull[/RESET]'."), "received message: "+commit.Error.Message)
+		return errs.Wrap(&CommitError{
+			commit.Type, commit.Error.Message,
+			locale.NewInputError("err_buildplanner_head_on_branch_moved"),
+		}, "received message: "+commit.Error.Message)
 	case NoChangeSinceLastCommitErrorType:
-		return errs.Wrap(locale.NewInputError("err_buildplanner_no_change_since_last_commit", "No new changes to commit."), commit.Error.Message)
+		return errs.Wrap(&CommitError{
+			commit.Type, commit.Error.Message,
+			locale.NewInputError("err_buildplanner_no_change_since_last_commit", "No new changes to commit."),
+		}, commit.Error.Message)
 	default:
 		return errs.New(fallbackMessage)
 	}

pkg/platform/runtime/buildexpression/buildexpression.go

@@ -767,6 +767,11 @@ func (e *BuildExpression) addRequirement(requirement model.Requirement) error {
 	return nil
 }
 
+type RequirementNotFoundError struct {
+	Name                   string
+	*locale.LocalizedError // for legacy non-user-facing error usages
+}
+
 func (e *BuildExpression) removeRequirement(requirement model.Requirement) error {
 	requirementsNode, err := e.getRequirementsNode()
 	if err != nil {
@@ -789,7 +794,10 @@ func (e *BuildExpression) removeRequirement(requirement model.Requirement) error
 	}
 
 	if !found {
-		return locale.NewInputError("err_remove_requirement_not_found", "Could not remove requirement '[ACTIONABLE]{{.V0}}[/RESET]', because it does not exist.", requirement.Name)
+		return &RequirementNotFoundError{
+			requirement.Name,
+			locale.NewInputError("err_remove_requirement_not_found", "", requirement.Name),
+		}
 	}
 
 	solveNode, err := e.getSolveNode()

test/integration/package_int_test.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"path/filepath"
 	"runtime"
+	"strings"
 	"testing"
 	"time"
 
@@ -398,9 +399,13 @@ func (suite *PackageIntegrationTestSuite) TestPackage_Duplicate() {
 	cp.ExpectExitCode(0)
 
 	cp = ts.Spawn("install", "requests") // install again
-	cp.Expect("No new changes to commit")
+	cp.Expect("already installed")
 	cp.ExpectNotExitCode(0)
 	ts.IgnoreLogErrors()
+
+	if strings.Count(cp.Snapshot(), " x ") != 2 { // 2 because "Creating commit x Failed" is also printed
+		suite.Fail("Expected exactly ONE error message, got: ", cp.Snapshot())
+	}
 }
 
 func (suite *PackageIntegrationTestSuite) PrepareActiveStateYAML(ts *e2e.Session) {
@@ -425,9 +430,13 @@ func (suite *PackageIntegrationTestSuite) TestPackage_UninstallDoesNotExist() {
 	suite.PrepareActiveStateYAML(ts)
 
 	cp := ts.Spawn("uninstall", "doesNotExist")
-	cp.Expect("Error occurred while trying to create a commit")
+	cp.Expect("does not exist")
 	cp.ExpectExitCode(1)
 	ts.IgnoreLogErrors()
+
+	if strings.Count(cp.Snapshot(), " x ") != 2 { // 2 because "Creating commit x Failed" is also printed
+		suite.Fail("Expected exactly ONE error message, got: ", cp.Snapshot())
+	}
 }
 
 func (suite *PackageIntegrationTestSuite) TestJSON() {