From 073cf922d745bc065f75bd4e7802858527935e4c Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Wed, 19 Aug 2026 11:44:08 +1000 Subject: [PATCH 1/4] fix: report missing package versions instead of a server null reference `release create --no-prompt` sends the create request straight to the server without resolving package versions first. When a package has no version in its feed the server raises a null reference exception, which surfaces as "Octopus API error: Object reference not set to an instance of an object. []". On a 5xx failure the CLI now repeats the package version resolution the server does, and reports the packages, steps and feeds that have no version available. Where it can't identify a specific package, an unhandled server error now carries a hint about the likely causes. Fixes #426 Co-Authored-By: Claude Opus 5 (1M context) --- pkg/cmd/release/create/create.go | 93 +++++++++++- pkg/cmd/release/create/create_test.go | 207 ++++++++++++++++++++++++++ pkg/packages/packages.go | 101 ++++++++++--- 3 files changed, 382 insertions(+), 19 deletions(-) diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index b4b50caa..2f1d835b 100644 --- a/pkg/cmd/release/create/create.go +++ b/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") diff --git a/pkg/cmd/release/create/create_test.go b/pkg/cmd/release/create/create_test.go index 65ee97c7..87078d8e 100644 --- a/pkg/cmd/release/create/create_test.go +++ b/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) + }) + } +} diff --git a/pkg/packages/packages.go b/pkg/packages/packages.go index 3eff889a..e32d0986 100644 --- a/pkg/packages/packages.go +++ b/pkg/packages/packages.go @@ -180,6 +180,88 @@ func BuildPackageVersionBaseline(octopus *octopusApiClient.Client, packages []re return result, nil } +// FindPackagesWithoutVersions returns the deployment process template packages which the server +// expects to have a version at release creation time, but for which no version could be found in the feed. +// Packages with a fixed version, or which aren't resolvable until deployment time, are excluded because +// they don't need one. +func FindPackagesWithoutVersions(templatePackages []releases.ReleaseTemplatePackage, resolvedVersions []*StepPackageVersion) []releases.ReleaseTemplatePackage { + result := make([]releases.ReleaseTemplatePackage, 0) + for _, templatePackage := range templatePackages { + if templatePackage.FixedVersion != "" || !templatePackage.IsResolvable { + continue + } + for _, resolved := range resolvedVersions { + if resolved.PackageID == templatePackage.PackageID && + resolved.ActionName == templatePackage.ActionName && + resolved.PackageReferenceName == templatePackage.PackageReferenceName { + if strings.TrimSpace(resolved.Version) == "" { + result = append(result, templatePackage) + } + break + } + } + } + return result +} + +// MissingPackageVersionsError is raised when one or more packages referenced by the deployment process +// have no version available in their feed. The server can't assemble a release in this state; rather than +// reporting that, it raises a null reference exception, so the CLI detects the situation itself. +type MissingPackageVersionsError struct { + Packages []releases.ReleaseTemplatePackage + cause error +} + +func NewMissingPackageVersionsError(missingPackages []releases.ReleaseTemplatePackage, cause error) *MissingPackageVersionsError { + return &MissingPackageVersionsError{Packages: missingPackages, cause: cause} +} + +func (e *MissingPackageVersionsError) Unwrap() error { return e.cause } + +func (e *MissingPackageVersionsError) Error() string { + sb := &strings.Builder{} + sb.WriteString("cannot create release; no version could be found for the following packages:") + for _, p := range e.Packages { + packageName := p.PackageID + if p.PackageReferenceName != "" && p.PackageReferenceName != p.PackageID { + packageName = fmt.Sprintf("%s/%s", packageName, p.PackageReferenceName) + } + feedName := p.FeedName + if feedName == "" { + feedName = p.FeedID + } + sb.WriteString(fmt.Sprintf("\n - '%s' in step '%s' (feed '%s')", packageName, p.ActionName, feedName)) + } + sb.WriteString("\npush the package(s) to the feed, or supply a version with --package or --package-version") + return sb.String() +} + +// BuildPackageVersionOverrides converts the --package-version and --package command line flags into +// resolved overrides, using the baseline to work out which step or package each override refers to. +// Anything that can't be parsed or resolved is ignored; the server reports those. +func BuildPackageVersionOverrides(packageVersionBaseline []*StepPackageVersion, defaultPackageVersion string, packageOverrideFlags []string) []*PackageVersionOverride { + packageVersionOverrides := make([]*PackageVersionOverride, 0, len(packageOverrideFlags)+1) + + if defaultPackageVersion != "" { + // blind apply to everything + packageVersionOverrides = append(packageVersionOverrides, &PackageVersionOverride{Version: defaultPackageVersion}) + } + + for _, s := range packageOverrideFlags { + ambOverride, err := ParsePackageOverrideString(s) + if err != nil { + continue // silently ignore anything that wasn't parseable (should we emit a warning?) + } + resolvedOverride, err := ResolvePackageOverride(ambOverride, packageVersionBaseline) + if err != nil { + continue // silently ignore anything that wasn't parseable (should we emit a warning?) + } + packageVersionOverrides = append(packageVersionOverrides, resolvedOverride) + } + + return packageVersionOverrides +} + type PackageVersionOverride struct { ActionName string // optional, but one or both of ActionName or PackageID must be supplied PackageID string // optional, but one or both of ActionName or PackageID must be supplied @@ -539,25 +621,8 @@ func AskPackageOverrideLoop( initialPackageOverrideFlags []string, // the --package command line flag (multiple occurrences) asker question.Asker, stdout io.Writer) ([]*StepPackageVersion, []*PackageVersionOverride, error) { - packageVersionOverrides := make([]*PackageVersionOverride, 0) - // pickup any partial package specifications that may have arrived on the commandline - if defaultPackageVersion != "" { - // blind apply to everything - packageVersionOverrides = append(packageVersionOverrides, &PackageVersionOverride{Version: defaultPackageVersion}) - } - - for _, s := range initialPackageOverrideFlags { - ambOverride, err := ParsePackageOverrideString(s) - if err != nil { - continue // silently ignore anything that wasn't parseable (should we emit a warning?) - } - resolvedOverride, err := ResolvePackageOverride(ambOverride, packageVersionBaseline) - if err != nil { - continue // silently ignore anything that wasn't parseable (should we emit a warning?) - } - packageVersionOverrides = append(packageVersionOverrides, resolvedOverride) - } + packageVersionOverrides := BuildPackageVersionOverrides(packageVersionBaseline, defaultPackageVersion, initialPackageOverrideFlags) overriddenPackageVersions := ApplyPackageOverrides(packageVersionBaseline, packageVersionOverrides) From ea972e2c9c8e3b5262e3cd1efb38c08539b8ca30 Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Mon, 31 Aug 2026 12:03:53 +1000 Subject: [PATCH 2/4] fix: only diagnose the null reference failure, not every 5xx The package diagnosis ran for any APIError with a 5xx status. On an unrelated server error that had the side effect of (a) replacing a real server message with MissingPackageVersionsError, whose Error() doesn't include the cause, and (b) firing ~6 extra requests at a server that is already failing. Require the null reference message before diagnosing, which is the only failure this code knows how to explain. The fallback hint no longer needs its own check, since reaching it now implies the message matched. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/cmd/release/create/create.go | 10 +++++----- pkg/cmd/release/create/create_test.go | 10 ++++++++++ 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index 2f1d835b..90ae84cb 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -429,8 +429,11 @@ const serverNullReferenceMessage = "Object reference not set to an instance of a // 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 { + // only the specific null reference failure is worth diagnosing. Any other 5xx is a real server error + // that we must report as-is; replacing it would hide the cause, and re-querying the server would pile + // more requests onto something that is already failing. var apiError *core.APIError - if !errors.As(cause, &apiError) || apiError.StatusCode < 500 { + if !errors.As(cause, &apiError) || apiError.StatusCode < 500 || !strings.Contains(apiError.ErrorMessage, serverNullReferenceMessage) { return cause } @@ -441,10 +444,7 @@ func DiagnoseCreateReleaseFailure(octopus *octopusApiClient.Client, options *exe } } - 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 + 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) } // findPackagesWithoutVersions repeats the package version resolution the server does when it assembles a diff --git a/pkg/cmd/release/create/create_test.go b/pkg/cmd/release/create/create_test.go index 87078d8e..ce04cbd9 100644 --- a/pkg/cmd/release/create/create_test.go +++ b/pkg/cmd/release/create/create_test.go @@ -2930,6 +2930,16 @@ func TestReleaseCreate_DiagnoseCreateReleaseFailure(t *testing.T) { badRequest := &core.APIError{ErrorMessage: "release version 1.0.0 already exists", StatusCode: http.StatusBadRequest} assert.Equal(t, error(badRequest), create.DiagnoseCreateReleaseFailure(nil, nil, badRequest)) }) + + t.Run("passes through server faults which aren't the null reference we know how to diagnose", func(t *testing.T) { + // an unrelated 5xx must be reported as-is; we mustn't replace it with a package diagnosis + // (nor go back to an already-failing server to run one) + serverError := &core.APIError{ErrorMessage: "The database is unavailable", StatusCode: http.StatusInternalServerError} + assert.Equal(t, error(serverError), create.DiagnoseCreateReleaseFailure(nil, nil, serverError)) + + badGateway := &core.APIError{ErrorMessage: "Bad Gateway", StatusCode: http.StatusBadGateway} + assert.Equal(t, error(badGateway), create.DiagnoseCreateReleaseFailure(nil, nil, badGateway)) + }) } // issue #426: the server raises a null reference exception rather than telling us that a package From 66ee41168b915eddd28ea72389a7bd37f4e9a193 Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Mon, 31 Aug 2026 12:05:36 +1000 Subject: [PATCH 3/4] fix: honour --ignore-channel-rules and channel IDs in the diagnosis Two ways the replay could diverge from what the server actually did: - With --ignore-channel-rules the server resolves package versions without applying the channel's version rules, but the replay always applied them. A package with versions in its feed, none satisfying the rules, would be reported as "no version could be found", misdiagnosing the real failure. Build the baseline without the rule filter in that case. - --channel reaches the server as ChannelIDOrName, but the lookup matched on name only, so passing a channel ID silently dropped the diagnosis to the generic hint. Match on either. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/cmd/release/create/create.go | 31 ++++++++++---- pkg/cmd/release/create/create_test.go | 59 +++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index 90ae84cb..5c7786de 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -478,7 +478,15 @@ func findPackagesWithoutVersions(octopus *octopusApiClient.Client, options *exec return nil, err } - packageVersionBaseline, err := BuildPackageVersionBaselineForChannel(octopus, deploymentProcessTemplate, channel) + // mirror what the server did: with --ignore-channel-rules it selects versions without applying the + // channel's version rules, so applying them here would report packages as missing when they only + // failed the rules. + var packageVersionBaseline []*packages.StepPackageVersion + if options.IgnoreChannelRules { + packageVersionBaseline, err = packages.BuildPackageVersionBaseline(octopus, deploymentProcessTemplate.Packages, nil) + } else { + packageVersionBaseline, err = BuildPackageVersionBaselineForChannel(octopus, deploymentProcessTemplate, channel) + } if err != nil { return nil, err } @@ -489,17 +497,24 @@ func findPackagesWithoutVersions(octopus *octopusApiClient.Client, options *exec 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) - } - +// findChannelForDiagnosis locates the channel the server would have used. --channel reaches the server as +// ChannelIDOrName, so we match on either. 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, channelIDOrName string) (*channels.Channel, error) { existingChannels, err := octopus.Projects.GetChannels(project) if err != nil { return nil, err } + + if channelIDOrName != "" { + for _, c := range existingChannels { + if strings.EqualFold(c.Name, channelIDOrName) || c.ID == channelIDOrName { + return c, nil + } + } + return nil, fmt.Errorf("no channel found with name or ID of %s", channelIDOrName) + } + if len(existingChannels) == 1 { return existingChannels[0], nil } diff --git a/pkg/cmd/release/create/create_test.go b/pkg/cmd/release/create/create_test.go index ce04cbd9..e7fee464 100644 --- a/pkg/cmd/release/create/create_test.go +++ b/pkg/cmd/release/create/create_test.go @@ -3010,6 +3010,65 @@ func TestReleaseCreate_AutomationMode_MissingPackageDiagnosis(t *testing.T) { assert.Equal(t, "", stdOut.String()) }}, + {"doesn't apply channel version rules when --ignore-channel-rules was specified", func(t *testing.T, api *testutil.MockHttpServer, rootCmd *cobra.Command, stdOut *bytes.Buffer, stdErr *bytes.Buffer) { + // the server resolved versions without the channel rules, so the diagnosis must too; + // otherwise a package which only fails the rules gets reported as having no version at all + ruledChannel := fixtures.NewChannel(spaceID, "Channels-1", "Default", fireProjectID) + ruledChannel.Rules = []channels.ChannelRule{{ + Tag: "^pre$", + VersionRange: "[5.0,6.0)", + ActionPackages: []octopusPackages.DeploymentActionPackage{ + {DeploymentAction: "Deploy Website", PackageReference: "acme-web"}, + }, + }} + + cmdReceiver := testutil.GoBegin2(func() (*cobra.Command, error) { + defer api.Close() + rootCmd.SetArgs([]string{"release", "create", "--project", fireProject.Name, "--ignore-channel-rules"}) + 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) + + 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{ruledChannel}, + }) + 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}", + }}}, + }}) + // no versionRange or preReleaseTag in the query, despite the channel carrying a rule for this package + 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`)) + }}, + {"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() From c12edf907ba98d6d2ab8830c8a74a13dbb39f078 Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Fri, 4 Sep 2026 14:23:46 +1000 Subject: [PATCH 4/4] fix: report the server's own error alongside the package diagnosis ea972e2 narrowed the diagnosis to failures carrying the server's null reference message, to stop an unrelated 5xx being reported as a package problem. That works, but it also switches the fix off on current servers: the #426 path there fails with "There are no viable release plans in any channels", not a null reference, so the message the server sends for this is version-dependent and can't be relied on as the trigger. Address the underlying complaint instead. MissingPackageVersionsError now prints what the server actually said, so a misattributed diagnosis costs the user a misleading paragraph rather than the real cause, which was previously reachable only via Unwrap and never printed (main.go prints err.Error() alone). With nothing hidden, the trigger widens back to any 5xx and keeps working across server versions. The null reference message itself is still suppressed from that output -- it says nothing the diagnosis doesn't say better -- so the integration test's guard against it resurfacing stays valid. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/cmd/release/create/create.go | 19 +++++++------ pkg/cmd/release/create/create_test.go | 13 +++++++-- pkg/packages/packages.go | 13 +++++++++ pkg/packages/packages_test.go | 40 +++++++++++++++++++++++++++ 4 files changed, 73 insertions(+), 12 deletions(-) create mode 100644 pkg/packages/packages_test.go diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index 5c7786de..3d3be1bb 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -421,19 +421,17 @@ 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 +// +// Any 5xx is diagnosed, not just the null reference one, because the message a server sends for this +// varies by version: current servers report "no viable release plans" instead. The cost of being wrong +// is bounded, since MissingPackageVersionsError reports what the server actually said alongside the +// diagnosis. func DiagnoseCreateReleaseFailure(octopus *octopusApiClient.Client, options *executor.TaskOptionsCreateRelease, cause error) error { - // only the specific null reference failure is worth diagnosing. Any other 5xx is a real server error - // that we must report as-is; replacing it would hide the cause, and re-querying the server would pile - // more requests onto something that is already failing. var apiError *core.APIError - if !errors.As(cause, &apiError) || apiError.StatusCode < 500 || !strings.Contains(apiError.ErrorMessage, serverNullReferenceMessage) { + if !errors.As(cause, &apiError) || apiError.StatusCode < 500 { return cause } @@ -444,7 +442,10 @@ func DiagnoseCreateReleaseFailure(octopus *octopusApiClient.Client, options *exe } } - 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) + if strings.Contains(apiError.ErrorMessage, packages.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 diff --git a/pkg/cmd/release/create/create_test.go b/pkg/cmd/release/create/create_test.go index e7fee464..77b85acc 100644 --- a/pkg/cmd/release/create/create_test.go +++ b/pkg/cmd/release/create/create_test.go @@ -2931,15 +2931,22 @@ func TestReleaseCreate_DiagnoseCreateReleaseFailure(t *testing.T) { assert.Equal(t, error(badRequest), create.DiagnoseCreateReleaseFailure(nil, nil, badRequest)) }) - t.Run("passes through server faults which aren't the null reference we know how to diagnose", func(t *testing.T) { - // an unrelated 5xx must be reported as-is; we mustn't replace it with a package diagnosis - // (nor go back to an already-failing server to run one) + t.Run("passes through server faults it cannot diagnose", func(t *testing.T) { + // a 5xx is only replaced when the CLI can positively name the packages behind it. With no + // client to go and look, and no null reference message to explain, the server error stands. serverError := &core.APIError{ErrorMessage: "The database is unavailable", StatusCode: http.StatusInternalServerError} assert.Equal(t, error(serverError), create.DiagnoseCreateReleaseFailure(nil, nil, serverError)) badGateway := &core.APIError{ErrorMessage: "Bad Gateway", StatusCode: http.StatusBadGateway} assert.Equal(t, error(badGateway), create.DiagnoseCreateReleaseFailure(nil, nil, badGateway)) }) + + t.Run("explains a bare null reference fault even when no packages are missing", func(t *testing.T) { + nullRef := &core.APIError{ErrorMessage: "Object reference not set to an instance of an object.", StatusCode: http.StatusInternalServerError} + err := create.DiagnoseCreateReleaseFailure(nil, nil, nullRef) + assert.ErrorIs(t, err, nullRef) + assert.Contains(t, err.Error(), "the server failed with an unhandled error") + }) } // issue #426: the server raises a null reference exception rather than telling us that a package diff --git a/pkg/packages/packages.go b/pkg/packages/packages.go index e32d0986..52f38eed 100644 --- a/pkg/packages/packages.go +++ b/pkg/packages/packages.go @@ -204,6 +204,11 @@ func FindPackagesWithoutVersions(templatePackages []releases.ReleaseTemplatePack return result } +// 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, so it is worth +// replacing rather than reporting. +const ServerNullReferenceMessage = "Object reference not set to an instance of an object" + // MissingPackageVersionsError is raised when one or more packages referenced by the deployment process // have no version available in their feed. The server can't assemble a release in this state; rather than // reporting that, it raises a null reference exception, so the CLI detects the situation itself. @@ -233,6 +238,14 @@ func (e *MissingPackageVersionsError) Error() string { sb.WriteString(fmt.Sprintf("\n - '%s' in step '%s' (feed '%s')", packageName, p.ActionName, feedName)) } sb.WriteString("\npush the package(s) to the feed, or supply a version with --package or --package-version") + // this diagnosis is inferred from a failure the server doesn't describe, so it can be wrong. + // Report what the server actually said too, unless that's the null reference message, which + // says nothing the lines above don't already say better. + if e.cause != nil { + if causeText := e.cause.Error(); !strings.Contains(causeText, ServerNullReferenceMessage) { + sb.WriteString(fmt.Sprintf("\nthe server reported: %s", causeText)) + } + } return sb.String() } diff --git a/pkg/packages/packages_test.go b/pkg/packages/packages_test.go new file mode 100644 index 00000000..c6e9a813 --- /dev/null +++ b/pkg/packages/packages_test.go @@ -0,0 +1,40 @@ +package packages_test + +import ( + "errors" + "testing" + + "github.com/OctopusDeploy/cli/pkg/packages" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/releases" + "github.com/stretchr/testify/assert" +) + +func TestMissingPackageVersionsError_Error(t *testing.T) { + missing := []releases.ReleaseTemplatePackage{ + {PackageID: "acme.web", ActionName: "Deploy Web", FeedID: "feeds-builtin", FeedName: "Octopus Server (built-in)"}, + } + + t.Run("names the package, the step and the feed", func(t *testing.T) { + err := packages.NewMissingPackageVersionsError(missing, nil) + assert.Contains(t, err.Error(), "no version could be found for the following packages") + assert.Contains(t, err.Error(), "'acme.web' in step 'Deploy Web' (feed 'Octopus Server (built-in)')") + assert.Contains(t, err.Error(), "push the package(s) to the feed") + }) + + // the diagnosis is inferred from a failure the server doesn't describe, so if we guessed wrong + // the user still needs to be able to see what actually went wrong + t.Run("reports what the server said alongside the diagnosis", func(t *testing.T) { + cause := errors.New("There are no viable release plans in any channels") + err := packages.NewMissingPackageVersionsError(missing, cause) + assert.Contains(t, err.Error(), "the server reported: There are no viable release plans in any channels") + assert.ErrorIs(t, err, cause) + }) + + t.Run("omits the null reference message, which explains nothing", func(t *testing.T) { + cause := errors.New("Octopus API error: " + packages.ServerNullReferenceMessage + " []") + err := packages.NewMissingPackageVersionsError(missing, cause) + assert.NotContains(t, err.Error(), packages.ServerNullReferenceMessage) + assert.NotContains(t, err.Error(), "the server reported") + assert.ErrorIs(t, err, cause) // still unwrappable, just not printed + }) +}