From 13cf6d0167cde7b220c4caf5e8abd65497cf7e04 Mon Sep 17 00:00:00 2001 From: Oscar Reimer Date: Tue, 19 Dec 2023 16:32:01 +0100 Subject: [PATCH] Add detailed error messaging to Composer resolution (#170) * Add detailed error messaging to Composer resolution * Clean-up switch statements --- internal/resolution/job/base_job.go | 10 +- internal/resolution/job/base_job_test.go | 4 +- internal/resolution/pm/composer/job.go | 126 +++++++++++++++++++- internal/resolution/pm/composer/job_test.go | 56 +++++++-- internal/resolution/pm/gomod/job.go | 4 +- internal/resolution/pm/gradle/job.go | 12 +- internal/resolution/pm/maven/job.go | 10 +- internal/resolution/pm/nuget/job.go | 2 +- internal/resolution/pm/pip/job.go | 10 +- internal/resolution/pm/yarn/job.go | 18 +-- scripts/install.sh | 1 + 11 files changed, 210 insertions(+), 43 deletions(-) diff --git a/internal/resolution/job/base_job.go b/internal/resolution/job/base_job.go index df07acc4..1ff82c91 100644 --- a/internal/resolution/job/base_job.go +++ b/internal/resolution/job/base_job.go @@ -35,11 +35,17 @@ func (j *BaseJob) SendStatus(status string) { j.status <- status } -func (j *BaseJob) GetExitError(err error) error { +func (j *BaseJob) GetExitError(err error, commandOutput string) error { exitErr, ok := err.(*exec.ExitError) if !ok { return err } - return errors.New(string(exitErr.Stderr)) + // If Stderr is empty, use commandOutput as error string instead + errorMessage := string(exitErr.Stderr) + if errorMessage == "" { + errorMessage = commandOutput + } + + return errors.New(errorMessage) } diff --git a/internal/resolution/job/base_job_test.go b/internal/resolution/job/base_job_test.go index bfff6895..5f741d3d 100644 --- a/internal/resolution/job/base_job_test.go +++ b/internal/resolution/job/base_job_test.go @@ -76,13 +76,13 @@ func TestGetExitErrorWithExitError(t *testing.T) { Stderr: []byte("stderr"), } j := BaseJob{} - exitErr := j.GetExitError(err) + exitErr := j.GetExitError(err, "") assert.ErrorContains(t, exitErr, string(err.Stderr)) } func TestGetExitErrorWithNoneExitError(t *testing.T) { err := &exec.Error{Err: errors.New("none-exit-err")} j := BaseJob{} - exitErr := j.GetExitError(err) + exitErr := j.GetExitError(err, "") assert.ErrorContains(t, exitErr, err.Error()) } diff --git a/internal/resolution/pm/composer/job.go b/internal/resolution/pm/composer/job.go index 3fd5f087..8710da99 100644 --- a/internal/resolution/pm/composer/job.go +++ b/internal/resolution/pm/composer/job.go @@ -1,12 +1,20 @@ package composer import ( + "regexp" + "strings" + "github.com/debricked/cli/internal/resolution/job" "github.com/debricked/cli/internal/resolution/pm/util" ) const ( - composer = "composer" + composer = "composer" + composerMissingExtension = "Composer requires it to run" + invalidRequirement = `require\.([^ ]*) is invalid, it should have a vendor name` + noNetworkRegex = `The following exception probably indicates you( are offline or)? have misconfigured DNS resolver\(s\)` + invalidVersionErrRegex = `requires\s+([^/]+/[^,]+),\s+found.*but it does not match the constraint\.` + dependenciesResolveErrRegex = `requires\s+([^/]+/[^,]+),\s+it\s+could\s+not\s+be\s+found\s+in\s+any\s+version` ) type Job struct { @@ -39,7 +47,7 @@ func (j *Job) Run() { _, err := j.runInstallCmd() if err != nil { cmdErr := util.NewPMJobError(err.Error()) - j.Errors().Critical(cmdErr) + j.handleError(cmdErr) return } @@ -48,7 +56,6 @@ func (j *Job) Run() { } func (j *Job) runInstallCmd() ([]byte, error) { - j.composerCommand = composer installCmd, err := j.cmdFactory.MakeInstallCmd(j.composerCommand, j.GetFile()) if err != nil { @@ -57,8 +64,119 @@ func (j *Job) runInstallCmd() ([]byte, error) { installCmdOutput, err := installCmd.Output() if err != nil { - return nil, j.GetExitError(err) + return nil, j.GetExitError(err, string(installCmdOutput)) } return installCmdOutput, nil } + +func (j *Job) handleError(cmdErr job.IError) { + expressions := []string{ + composerMissingExtension, + invalidRequirement, + noNetworkRegex, + invalidVersionErrRegex, + dependenciesResolveErrRegex, + } + + for _, expression := range expressions { + regex := regexp.MustCompile(expression) + matches := regex.FindAllStringSubmatch(cmdErr.Error(), -1) + + if len(matches) > 0 { + cmdErr = j.addDocumentation(expression, regex, matches, cmdErr) + j.Errors().Critical(cmdErr) + + return + } + } + + j.Errors().Critical(cmdErr) +} + +func (j *Job) addDocumentation(expr string, regex *regexp.Regexp, matches [][]string, cmdErr job.IError) job.IError { + documentation := cmdErr.Documentation() + + switch expr { + case composerMissingExtension: + documentation = j.addComposerMissingRequirementsErrorDocumentation(cmdErr) + case invalidRequirement: + documentation = j.addInvalidRequirementErrorDocumentation(matches) + case noNetworkRegex: + documentation = j.addNetworkUnreachableErrorDocumentation() + case invalidVersionErrRegex: + documentation = j.addInvalidVersionErrorDocumentation(matches) + case dependenciesResolveErrRegex: + documentation = j.addDependenciesResolveErrorDocumentation(matches) + } + + cmdErr.SetDocumentation(documentation) + + return cmdErr +} + +func (j *Job) addComposerMissingRequirementsErrorDocumentation(cmdErr job.IError) string { + return strings.Join( + []string{ + "Failed to build Composer dependency tree.", + "Your runtime environment is missing one or more Composer requirements.", + "Check error message below for more details:\n\n", + cmdErr.Error(), + }, " ") +} + +func (j *Job) addInvalidRequirementErrorDocumentation(matches [][]string) string { + message := "" + if len(matches) > 0 && len(matches[0]) > 1 { + message = matches[0][1] + } + + return strings.Join( + []string{ + "Couldn't resolve dependency", + message, + ", please make sure it is spelt correctly:\n", + }, " ") +} + +func (j *Job) addNetworkUnreachableErrorDocumentation() string { + return strings.Join( + []string{ + "We weren't able to retrieve one or more dependencies.", + "Please check your Internet connection and try again.", + }, " ") +} + +func (j *Job) addInvalidVersionErrorDocumentation(matches [][]string) string { + message := "" + if len(matches) > 0 && len(matches[0]) > 1 { + message = matches[0][1] + } + + return strings.Join( + []string{ + "Couldn't resolve version", + message, + ", please make sure it exists:\n", + }, " ") +} + +func (j *Job) addDependenciesResolveErrorDocumentation(matches [][]string) string { + message := "An error occurred during dependencies resolve " + if len(matches) > 0 && len(matches[0]) > 1 { + message += strings.Join( + []string{ + "for: ", + matches[0][1], + "", + }, "") + } + + return strings.Join( + []string{ + message, + "\n\n", + util.InstallPrivateDependencyMessage, + "\n\n", + }, "") +} diff --git a/internal/resolution/pm/composer/job_test.go b/internal/resolution/pm/composer/job_test.go index ee6ae4fa..9dd23399 100644 --- a/internal/resolution/pm/composer/job_test.go +++ b/internal/resolution/pm/composer/job_test.go @@ -6,6 +6,7 @@ import ( jobTestdata "github.com/debricked/cli/internal/resolution/job/testdata" "github.com/debricked/cli/internal/resolution/pm/composer/testdata" + "github.com/debricked/cli/internal/resolution/pm/util" "github.com/stretchr/testify/assert" ) @@ -40,15 +41,56 @@ func TestInstall(t *testing.T) { } func TestRunInstallCmdErr(t *testing.T) { - cmdErr := errors.New("cmd-error") - cmdFactoryMock := testdata.NewEchoCmdFactory() - cmdFactoryMock.MakeInstallErr = cmdErr - j := NewJob("file", true, cmdFactoryMock) + cases := []struct { + error string + doc string + }{ + { + error: "cmd-error", + doc: util.UnknownError, + }, + { + error: "\n\n PHP's phar extension is missing. Composer requires it to run. Enable the extension or recompile php without --disable-phar then try again.", + doc: "Failed to build Composer dependency tree. Your runtime environment is missing one or more Composer requirements. Check error message below for more details:\n\n \n\n PHP's phar extension is missing. Composer requires it to run. Enable the extension or recompile php without --disable-phar then try again.", + }, + { + error: "require.debricked is invalid, it should have a vendor name, a forward slash, and a package name", + doc: "Couldn't resolve dependency debricked , please make sure it is spelt correctly:\n", + }, + { + error: "The following exception probably indicates you have misconfigured DNS resolver(s)\n\n[Composer\\Downloader\\TransportException]\ncurl error 6 while downloading https://flex.symfony.com/versions.json: Could not resolve host: flex.symfony.com", + doc: "We weren't able to retrieve one or more dependencies. Please check your Internet connection and try again.", + }, + { + error: "The following exception probably indicates you are offline or have misconfigured DNS resolver(s)\n\n[Composer\\Downloader\\TransportException]\ncurl error 6 while downloading https://flex.symfony.com/versions.json: Could not resolve host: flex.symfony.com", + doc: "We weren't able to retrieve one or more dependencies. Please check your Internet connection and try again.", + }, + { + error: "Root composer.json requires drupal/entity_pager 1.0@RC, found drupal/entity_pager[dev-1.x, dev-2.0.x, 1.0.0-alpha1, ..., 1.x-dev (alias of dev-1.x), 2.0.x-dev (alias of dev-2.0.x)] but it does not match the constraint.", + doc: "Couldn't resolve version drupal/entity_pager 1.0@RC , please make sure it exists:\n", + }, + { + error: "Loading composer repositories with package information\nUpdating dependencies\nYour requirements could not be resolved to an installable set of packages.\n\n Problem 1\n - Root composer.json requires blablabla/blabla, it could not be found in any version, there may be a typo in the package name.\n\nPotential causes:\n - A typo in the package name\n - The package is not available in a stable-enough version according to your minimum-stability setting\n see for more details.\n - It's a private package and you forgot to add a custom repository to find it\n\nRead for further common problems.\n", + doc: "An error occurred during dependencies resolve for: blablabla/blabla\n\nIf this is a private dependency, please make sure that the debricked CLI has access to install it or pre-install it before running the debricked CLI.\n\n", + }, + } - go jobTestdata.WaitStatus(j) - j.Run() + for _, c := range cases { + expectedError := util.NewPMJobError(c.error) + expectedError.SetDocumentation(c.doc) + + cmdErr := errors.New(c.error) + j := NewJob("file", true, testdata.CmdFactoryMock{InstallCmdName: "echo", MakeInstallErr: cmdErr}) + + go jobTestdata.WaitStatus(j) + + j.Run() + + errors := j.Errors().GetAll() - assert.Len(t, j.Errors().GetAll(), 1) + assert.Len(t, errors, 1) + assert.Contains(t, errors, expectedError) + } } func TestRunInstallCmdOutputErr(t *testing.T) { diff --git a/internal/resolution/pm/gomod/job.go b/internal/resolution/pm/gomod/job.go index 67e7245f..a85c0e95 100644 --- a/internal/resolution/pm/gomod/job.go +++ b/internal/resolution/pm/gomod/job.go @@ -78,7 +78,7 @@ func (j *Job) runGraphCmd(workingDirectory string) ([]byte, error) { graphCmdOutput, err := graphCmd.Output() if err != nil { - return nil, j.GetExitError(err) + return nil, j.GetExitError(err, "") } return graphCmdOutput, nil @@ -92,7 +92,7 @@ func (j *Job) runListCmd(workingDirectory string) ([]byte, error) { listCmdOutput, err := listCmd.Output() if err != nil { - return nil, j.GetExitError(err) + return nil, j.GetExitError(err, "") } return listCmdOutput, nil diff --git a/internal/resolution/pm/gradle/job.go b/internal/resolution/pm/gradle/job.go index ae662453..072ad914 100644 --- a/internal/resolution/pm/gradle/job.go +++ b/internal/resolution/pm/gradle/job.go @@ -81,7 +81,7 @@ func (j *Job) Run() { } if err != nil { - cmdErr := util.NewPMJobError(j.GetExitError(err).Error()) + cmdErr := util.NewPMJobError(j.GetExitError(err, "").Error()) cmdErr.SetCommand(dependenciesCmd.String()) j.handleError(cmdErr) @@ -116,14 +116,14 @@ func (j *Job) handleError(cmdErr job.IError) { } func (j *Job) addDocumentation(expr string, regex *regexp.Regexp, cmdErr job.IError) job.IError { - switch { - case expr == bugErrRegex: + switch expr { + case bugErrRegex: cmdErr = j.addBugErrorDocumentation(regex, cmdErr) - case expr == notRootDirErrRegex: + case notRootDirErrRegex: cmdErr = j.addNotRootDirErrorDocumentation(regex, cmdErr) - case expr == unrelatedBuildErrRegex: + case unrelatedBuildErrRegex: cmdErr = j.addUnrelatedBuildErrorDocumentation(regex, cmdErr) - case expr == unknownPropertyErrRegex: + case unknownPropertyErrRegex: cmdErr = j.addUnknownPropertyErrorDocumentation(regex, cmdErr) } diff --git a/internal/resolution/pm/maven/job.go b/internal/resolution/pm/maven/job.go index 9e42bebb..b1c9dd23 100644 --- a/internal/resolution/pm/maven/job.go +++ b/internal/resolution/pm/maven/job.go @@ -78,14 +78,14 @@ func (j *Job) handleError(cmdErr job.IError) { } func (j *Job) addDocumentation(expr string, regex *regexp.Regexp, cmdErr job.IError) job.IError { - switch { - case expr == nonParseablePomErrRegex: + switch expr { + case nonParseablePomErrRegex: cmdErr = j.addNonParseablePomErrorDocumentation(regex, cmdErr) - case expr == networkUnreachableErrRegex: + case networkUnreachableErrRegex: cmdErr = j.addNetworkUnreachableErrorDocumentation(cmdErr) - case expr == invalidVersionErrRegex: + case invalidVersionErrRegex: cmdErr = j.addInvalidVersionErrorDocumentation(regex, cmdErr) - case expr == dependenciesResolveErrRegex: + case dependenciesResolveErrRegex: cmdErr = j.addDependenciesResolveErrorDocumentation(regex, cmdErr) } diff --git a/internal/resolution/pm/nuget/job.go b/internal/resolution/pm/nuget/job.go index b9b4b973..27705794 100644 --- a/internal/resolution/pm/nuget/job.go +++ b/internal/resolution/pm/nuget/job.go @@ -63,7 +63,7 @@ func (j *Job) runInstallCmd() ([]byte, error) { installCmdOutput, err := installCmd.Output() if err != nil { - return installCmdOutput, j.GetExitError(err) + return installCmdOutput, j.GetExitError(err, "") } return installCmdOutput, nil diff --git a/internal/resolution/pm/pip/job.go b/internal/resolution/pm/pip/job.go index ffde27d5..38c2f159 100644 --- a/internal/resolution/pm/pip/job.go +++ b/internal/resolution/pm/pip/job.go @@ -237,7 +237,7 @@ func (j *Job) runCreateVenvCmd() ([]byte, job.IError) { createVenvCmdOutput, err := createVenvCmd.Output() if err != nil { - cmdErr := util.NewPMJobError(j.GetExitError(err).Error()) + cmdErr := util.NewPMJobError(j.GetExitError(err, "").Error()) cmdErr.SetCommand(createVenvCmd.String()) return nil, cmdErr @@ -268,7 +268,7 @@ func (j *Job) runInstallCmd() ([]byte, job.IError) { installCmdOutput, err := installCmd.Output() if err != nil { - cmdErr := util.NewPMJobError(j.GetExitError(err).Error()) + cmdErr := util.NewPMJobError(j.GetExitError(err, "").Error()) cmdErr.SetCommand(installCmd.String()) return nil, cmdErr @@ -288,7 +288,7 @@ func (j *Job) runCatCmd() ([]byte, job.IError) { listCmdOutput, err := listCmd.Output() if err != nil { - cmdErr := util.NewPMJobError(j.GetExitError(err).Error()) + cmdErr := util.NewPMJobError(j.GetExitError(err, "").Error()) cmdErr.SetCommand(listCmd.String()) return nil, cmdErr @@ -308,7 +308,7 @@ func (j *Job) runListCmd() ([]byte, job.IError) { listCmdOutput, err := listCmd.Output() if err != nil { - cmdErr := util.NewPMJobError(j.GetExitError(err).Error()) + cmdErr := util.NewPMJobError(j.GetExitError(err, "").Error()) cmdErr.SetCommand(listCmd.String()) return nil, cmdErr @@ -328,7 +328,7 @@ func (j *Job) runShowCmd(packages []string) ([]byte, job.IError) { listCmdOutput, err := listCmd.Output() if err != nil { - cmdErr := util.NewPMJobError(j.GetExitError(err).Error()) + cmdErr := util.NewPMJobError(j.GetExitError(err, "").Error()) cmdErr.SetCommand(listCmd.String()) return nil, cmdErr diff --git a/internal/resolution/pm/yarn/job.go b/internal/resolution/pm/yarn/job.go index 16558b2f..cd239872 100644 --- a/internal/resolution/pm/yarn/job.go +++ b/internal/resolution/pm/yarn/job.go @@ -57,7 +57,7 @@ func (j *Job) Run() { } if output, err := installCmd.Output(); err != nil { - error := strings.Join([]string{string(output), j.GetExitError(err).Error()}, "") + error := strings.Join([]string{string(output), j.GetExitError(err, "").Error()}, "") j.handleError(j.createError(error, installCmd.String(), status)) return @@ -102,20 +102,20 @@ func (j *Job) handleError(cmdError job.IError) { func (j *Job) addDocumentation(expr string, matches [][]string, cmdError job.IError) job.IError { documentation := cmdError.Documentation() - switch { - case expr == invalidJsonErrRegex: + switch expr { + case invalidJsonErrRegex: documentation = getInvalidJsonErrorDocumentation(matches) - case expr == invalidSchemaErrRegex: + case invalidSchemaErrRegex: documentation = getInvalidSchemaErrorDocumentation(matches) - case expr == invalidArgumentErrRegex: + case invalidArgumentErrRegex: documentation = getInvalidArgumentErrorDocumentation(matches) - case expr == versionNotFoundErrRegex: + case versionNotFoundErrRegex: documentation = getVersionNotFoundErrorDocumentation(matches) - case expr == dependencyNotFoundErrRegex: + case dependencyNotFoundErrRegex: documentation = getDependencyNotFoundErrorDocumentation(matches) - case expr == registryUnavailableErrRegex: + case registryUnavailableErrRegex: documentation = getRegistryUnavailableErrorDocumentation(matches) - case expr == permissionDeniedErrRegex: + case permissionDeniedErrRegex: documentation = getPermissionDeniedErrorDocumentation(matches) } diff --git a/scripts/install.sh b/scripts/install.sh index 4de5b9ef..267048b9 100644 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -7,3 +7,4 @@ fi version=$(git symbolic-ref -q --short HEAD || git describe --tags --exact-match) ldFlags="-X main.version=${version}" go install -ldflags "${ldFlags}" ./cmd/debricked +go generate -v -x ./cmd/debricked