fix: report missing package versions instead of a server null reference

pkg/cmd/release/create/create.go

@@ -28,6 +28,7 @@ import (
 	"github.com/OctopusDeploy/cli/pkg/util/flag"
 	"github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/channels"
 	octopusApiClient "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/client"
+	"github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/core"
 	"github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/deployments"
 	"github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/feeds"
 	"github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/projects"
@@ -318,7 +319,7 @@ func createRun(cmd *cobra.Command, f factory.Factory, flags *CreateFlags) error
 		executor.NewTask(executor.TaskTypeCreateRelease, options),
 	})
 	if err != nil {
-		return err
+		return DiagnoseCreateReleaseFailure(octopus, options, err)
 	}
 
 	if options.Response != nil {
@@ -420,6 +421,96 @@ func BuildPackageVersionBaselineForChannel(octopus *octopusApiClient.Client, dep
 	return result, nil
 }
 
+// serverNullReferenceMessage is what an Octopus Server sends back when it hits an unhandled
+// null reference exception; it carries no information about what actually went wrong.
+const serverNullReferenceMessage = "Object reference not set to an instance of an object"
+
+// DiagnoseCreateReleaseFailure replaces an opaque server-side failure with an actionable message where
+// it can. The server raises a null reference exception, surfaced as a bare 500, when it can't select a
+// version for a package; see https://github.com/OctopusDeploy/cli/issues/426
+func DiagnoseCreateReleaseFailure(octopus *octopusApiClient.Client, options *executor.TaskOptionsCreateRelease, cause error) error {
+	var apiError *core.APIError
+	if !errors.As(cause, &apiError) || apiError.StatusCode < 500 {
+		return cause
+	}
+
+	// diagnosis is best-effort; if any part of it fails we must not mask the original failure
+	if octopus != nil && options != nil {
+		if missingPackages, findErr := findPackagesWithoutVersions(octopus, options); findErr == nil && len(missingPackages) > 0 {
+			return packages.NewMissingPackageVersionsError(missingPackages, cause)
+		}
+	}
+
+	if strings.Contains(apiError.ErrorMessage, serverNullReferenceMessage) {
+		return fmt.Errorf("%w\nthe server failed with an unhandled error; this usually means it could not resolve the packages, channel or git reference for the release", cause)
+	}
+	return cause
+}
+
+// findPackagesWithoutVersions repeats the package version resolution the server does when it assembles a
+// release, so we can report which packages have no version available in their feed.
+func findPackagesWithoutVersions(octopus *octopusApiClient.Client, options *executor.TaskOptionsCreateRelease) ([]releases.ReleaseTemplatePackage, error) {
+	project, err := selectors.FindProject(octopus, options.ProjectName)
+	if err != nil {
+		return nil, err
+	}
+
+	gitReferenceKey := ""
+	if project.PersistenceSettings != nil && project.PersistenceSettings.Type() == projects.PersistenceSettingsTypeVersionControlled {
+		gitReferenceKey = options.GitReference
+		if options.GitCommit != "" { // prefer a specific git commit if one was specified
+			gitReferenceKey = options.GitCommit
+		}
+	}
+
+	deploymentProcess, err := octopus.DeploymentProcesses.Get(project, gitReferenceKey)
+	if err != nil {
+		return nil, err
+	}
+
+	channel, err := findChannelForDiagnosis(octopus, project, options.ChannelName)
+	if err != nil {
+		return nil, err
+	}
+
+	deploymentProcessTemplate, err := octopus.DeploymentProcesses.GetTemplate(deploymentProcess, channel.ID, "")
+	if err != nil {
+		return nil, err
+	}
+
+	packageVersionBaseline, err := BuildPackageVersionBaselineForChannel(octopus, deploymentProcessTemplate, channel)
+	if err != nil {
+		return nil, err
+	}
+
+	overrides := packages.BuildPackageVersionOverrides(packageVersionBaseline, options.DefaultPackageVersion, options.PackageVersionOverrides)
+	resolvedVersions := packages.ApplyPackageOverrides(packageVersionBaseline, overrides)
+
+	return packages.FindPackagesWithoutVersions(deploymentProcessTemplate.Packages, resolvedVersions), nil
+}
+
+// findChannelForDiagnosis locates the channel the server would have used. When no channel was specified we
+// can only guess; the default channel is the best approximation available to us.
+func findChannelForDiagnosis(octopus *octopusApiClient.Client, project *projects.Project, channelName string) (*channels.Channel, error) {
+	if channelName != "" {
+		return selectors.FindChannel(octopus, project, channelName)
+	}
+
+	existingChannels, err := octopus.Projects.GetChannels(project)
+	if err != nil {
+		return nil, err
+	}
+	if len(existingChannels) == 1 {
+		return existingChannels[0], nil
+	}
+	for _, c := range existingChannels {
+		if c.IsDefault {
+			return c, nil
+		}
+	}
+	return nil, fmt.Errorf("cannot determine the default channel for project %s", project.GetName())
+}
+
 func AskQuestions(octopus *octopusApiClient.Client, stdout io.Writer, asker question.Asker, options *executor.TaskOptionsCreateRelease) error {
 	if octopus == nil {
 		return cliErrors.NewArgumentNullOrEmptyError("octopus")

pkg/cmd/release/create/create_test.go

@@ -3,6 +3,7 @@ package create_test
 import (
 	"bytes"
 	"errors"
+	"net/http"
 	"net/url"
 	"os"
 	"testing"
@@ -19,6 +20,7 @@ import (
 	"github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/channels"
 	octopusApiClient "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/client"
 	"github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/constants"
+	"github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/core"
 	"github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/credentials"
 	"github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/deployments"
 	"github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/feeds"
@@ -2829,3 +2831,208 @@ func TestReleaseCreate_ApplyPackageOverride(t *testing.T) {
 		}, result)
 	})
 }
+
+func TestReleaseCreate_FindPackagesWithoutVersions(t *testing.T) {
+	resolvable := releases.ReleaseTemplatePackage{
+		ActionName:           "Deploy Website",
+		FeedID:               "feeds-builtin",
+		FeedName:             "Octopus Server (built-in)",
+		PackageID:            "acme-web",
+		PackageReferenceName: "acme-web",
+		IsResolvable:         true,
+	}
+
+	t.Run("reports a resolvable package with no version", func(t *testing.T) {
+		missing := packages.FindPackagesWithoutVersions(
+			[]releases.ReleaseTemplatePackage{resolvable},
+			[]*packages.StepPackageVersion{{PackageID: "acme-web", ActionName: "Deploy Website", PackageReferenceName: "acme-web", Version: ""}})
+
+		assert.Equal(t, []releases.ReleaseTemplatePackage{resolvable}, missing)
+	})
+
+	t.Run("ignores a package which has a version", func(t *testing.T) {
+		missing := packages.FindPackagesWithoutVersions(
+			[]releases.ReleaseTemplatePackage{resolvable},
+			[]*packages.StepPackageVersion{{PackageID: "acme-web", ActionName: "Deploy Website", PackageReferenceName: "acme-web", Version: "1.0.0"}})
+
+		assert.Equal(t, []releases.ReleaseTemplatePackage{}, missing)
+	})
+
+	t.Run("ignores packages which don't need a version at release creation time", func(t *testing.T) {
+		fixed := resolvable
+		fixed.FixedVersion = "1.0.0"
+		unresolvable := resolvable
+		unresolvable.IsResolvable = false
+
+		missing := packages.FindPackagesWithoutVersions(
+			[]releases.ReleaseTemplatePackage{fixed, unresolvable},
+			[]*packages.StepPackageVersion{{PackageID: "acme-web", ActionName: "Deploy Website", PackageReferenceName: "acme-web", Version: ""}})
+
+		assert.Equal(t, []releases.ReleaseTemplatePackage{}, missing)
+	})
+
+	t.Run("matches on step and package reference, not just package ID", func(t *testing.T) {
+		secondStep := resolvable
+		secondStep.ActionName = "Deploy Worker"
+
+		missing := packages.FindPackagesWithoutVersions(
+			[]releases.ReleaseTemplatePackage{resolvable, secondStep},
+			[]*packages.StepPackageVersion{
+				{PackageID: "acme-web", ActionName: "Deploy Website", PackageReferenceName: "acme-web", Version: "1.0.0"},
+				{PackageID: "acme-web", ActionName: "Deploy Worker", PackageReferenceName: "acme-web", Version: ""},
+			})
+
+		assert.Equal(t, []releases.ReleaseTemplatePackage{secondStep}, missing)
+	})
+}
+
+func TestReleaseCreate_MissingPackageVersionsError(t *testing.T) {
+	cause := errors.New("Octopus API error: Object reference not set to an instance of an object. []")
+
+	t.Run("names the package, step and feed", func(t *testing.T) {
+		err := packages.NewMissingPackageVersionsError([]releases.ReleaseTemplatePackage{{
+			ActionName:           "Deploy Website",
+			FeedID:               "feeds-builtin",
+			FeedName:             "Octopus Server (built-in)",
+			PackageID:            "acme-web",
+			PackageReferenceName: "acme-web",
+		}}, cause)
+
+		assert.EqualError(t, err, heredoc.Doc(`
+			cannot create release; no version could be found for the following packages:
+			  - 'acme-web' in step 'Deploy Website' (feed 'Octopus Server (built-in)')
+			push the package(s) to the feed, or supply a version with --package or --package-version`))
+
+		assert.Equal(t, cause, errors.Unwrap(err))
+	})
+
+	t.Run("qualifies the package with its reference name where they differ", func(t *testing.T) {
+		err := packages.NewMissingPackageVersionsError([]releases.ReleaseTemplatePackage{{
+			ActionName:           "Deploy Website",
+			FeedID:               "Feeds-1001",
+			PackageID:            "acme-web",
+			PackageReferenceName: "extra-config",
+		}}, cause)
+
+		// no FeedName in this response, so it falls back to the feed ID
+		assert.EqualError(t, err, heredoc.Doc(`
+			cannot create release; no version could be found for the following packages:
+			  - 'acme-web/extra-config' in step 'Deploy Website' (feed 'Feeds-1001')
+			push the package(s) to the feed, or supply a version with --package or --package-version`))
+	})
+}
+
+func TestReleaseCreate_DiagnoseCreateReleaseFailure(t *testing.T) {
+	t.Run("passes through errors which aren't server faults", func(t *testing.T) {
+		cause := errors.New("no such host")
+		assert.Equal(t, cause, create.DiagnoseCreateReleaseFailure(nil, nil, cause))
+
+		badRequest := &core.APIError{ErrorMessage: "release version 1.0.0 already exists", StatusCode: http.StatusBadRequest}
+		assert.Equal(t, error(badRequest), create.DiagnoseCreateReleaseFailure(nil, nil, badRequest))
+	})
+}
+
+// issue #426: the server raises a null reference exception rather than telling us that a package
+// referenced by the deployment process has no version available in its feed
+func TestReleaseCreate_AutomationMode_MissingPackageDiagnosis(t *testing.T) {
+	const spaceID = "Spaces-1"
+	const fireProjectID = "Projects-22"
+	const builtinFeedID = "feeds-builtin"
+
+	space1 := fixtures.NewSpace(spaceID, "Default Space")
+	depProcess := fixtures.NewDeploymentProcessForProject(spaceID, fireProjectID)
+	fireProject := fixtures.NewProject(spaceID, fireProjectID, "Fire Project", "Lifecycles-1", "ProjectGroups-1", depProcess.ID)
+	defaultChannel := fixtures.NewChannel(spaceID, "Channels-1", "Default", fireProjectID)
+
+	nullReferenceError := &core.APIError{ErrorMessage: "Object reference not set to an instance of an object."}
+
+	tests := []struct {
+		name string
+		run  func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer)
+	}{
+		{"reports the package which has no version in its feed", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) {
+			cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) {
+				defer api.Close()
+				rootCmd.SetArgs([]string{"release", "create", "--project", fireProject.Name, "--version", "1.0.0"})
+				return rootCmd.ExecuteC()
+			})
+
+			api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource)
+			api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource)
+			api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWith(fireProject)
+
+			api.ExpectRequest(t, "POST", "/api/Spaces-1/releases/create/v1").
+				RespondWithStatus(http.StatusInternalServerError, "500 Internal Server Error", nullReferenceError)
+
+			// the CLI now goes back to the server to work out what the real problem was
+			api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWith(fireProject)
+			api.ExpectRequest(t, "GET", "/api/Spaces-1/deploymentprocesses/"+depProcess.ID).RespondWith(depProcess)
+			api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/channels").RespondWith(resources.Resources[*channels.Channel]{
+				Items: []*channels.Channel{defaultChannel},
+			})
+			api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/"+fireProjectID+"/deploymentprocesses/template?channel=Channels-1").
+				RespondWith(&deployments.DeploymentProcessTemplate{
+					Packages: []releases.ReleaseTemplatePackage{{
+						ActionName:           "Deploy Website",
+						FeedID:               builtinFeedID,
+						FeedName:             "Octopus Server (built-in)",
+						PackageID:            "acme-web",
+						PackageReferenceName: "acme-web",
+						IsResolvable:         true,
+					}},
+				})
+			api.ExpectRequest(t, "GET", "/api/Spaces-1/feeds?ids="+builtinFeedID+"&take=1").RespondWith(&feeds.Feeds{Items: []feeds.IFeed{
+				&feeds.FeedResource{Name: "Octopus Server (built-in)", FeedType: feeds.FeedTypeBuiltIn, Resource: resources.Resource{
+					ID: builtinFeedID,
+					Links: map[string]string{
+						constants.LinkSearchPackageVersionsTemplate: "/api/Spaces-1/feeds/feeds-builtin/packages/versions{?packageId,take,skip,includePreRelease,versionRange,preReleaseTag,filter,includeReleaseNotes}",
+					}}},
+			}})
+			api.ExpectRequest(t, "GET", "/api/Spaces-1/feeds/feeds-builtin/packages/versions?packageId=acme-web&take=1").
+				RespondWith(&resources.Resources[*octopusPackages.PackageVersion]{Items: []*octopusPackages.PackageVersion{}})
+
+			_, err := testutil.ReceivePair(cmdReceiver)
+			assert.EqualError(t, err, heredoc.Doc(`
+				cannot create release; no version could be found for the following packages:
+				  - 'acme-web' in step 'Deploy Website' (feed 'Octopus Server (built-in)')
+				push the package(s) to the feed, or supply a version with --package or --package-version`))
+
+			assert.Equal(t, "", stdOut.String())
+		}},
+
+		{"falls back to a hint when it can't identify a missing package", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) {
+			cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) {
+				defer api.Close()
+				rootCmd.SetArgs([]string{"release", "create", "--project", fireProject.Name})
+				return rootCmd.ExecuteC()
+			})
+
+			api.ExpectRequest(t, "GET", "/api/").RespondWith(rootResource)
+			api.ExpectRequest(t, "GET", "/api/Spaces-1").RespondWith(rootResource)
+			api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWith(fireProject)
+
+			api.ExpectRequest(t, "POST", "/api/Spaces-1/releases/create/v1").
+				RespondWithStatus(http.StatusInternalServerError, "500 Internal Server Error", nullReferenceError)
+
+			// the diagnosis is best-effort; this server can't tell us about the deployment process
+			api.ExpectRequest(t, "GET", "/api/Spaces-1/projects/Fire Project").RespondWith(fireProject)
+			api.ExpectRequest(t, "GET", "/api/Spaces-1/deploymentprocesses/"+depProcess.ID).RespondWithStatus(http.StatusNotFound, "404 Not Found", nil)
+
+			_, err := testutil.ReceivePair(cmdReceiver)
+			assert.EqualError(t, err, "Octopus API error: Object reference not set to an instance of an object. [] \nthe server failed with an unhandled error; this usually means it could not resolve the packages, channel or git reference for the release")
+		}},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			stdout, stderr := &bytes.Buffer{}, &bytes.Buffer{}
+			api := testutil.NewMockHttpServer()
+
+			rootCmd := cmdRoot.NewCmdRoot(testutil.NewMockFactoryWithSpace(api, space1), nil, nil)
+			rootCmd.SetOut(stdout)
+			rootCmd.SetErr(stderr)
+
+			test.run(t, api, rootCmd, stdout, stderr)
+		})
+	}
+}