fix: stop identity delete command printing internal sentinel error

cmd/identities/delete.go

@@ -4,6 +4,7 @@
 package identities
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/spf13/cobra"
@@ -26,7 +27,7 @@ func NewDeleteCmd() *cobra.Command {
 
 func NewDeleteIdentityCmd() *cobra.Command {
 	return &cobra.Command{
-		Use:   "identity id-0 [id-1] [id-2] [id-n]",
+		Use:   "identity <id> [<id> ...]",
 		Short: "Delete one or more identities by their ID(s)",
 		Long: fmt.Sprintf(`This command deletes one or more identities by ID. To delete an identity by some selector, e.g. the recovery email address, use the list command in combination with jq.
 
@@ -43,13 +44,22 @@ func NewDeleteIdentityCmd() *cobra.Command {
 
 			var (
 				deleted = make([]cmdx.OutputIder, 0, len(args))
-				failed  = make(map[string]error)
+				failed  []string
 			)
 
 			for _, a := range args {
 				_, err := c.IdentityAPI.DeleteIdentity(cmd.Context(), a).Execute()
 				if err != nil {
-					failed[a] = cmdx.PrintOpenAPIError(cmd, err)
+					_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Failed to delete identity %s:\n", a)
+					// PrintOpenAPIError prints the message itself and returns the
+					// ErrNoPrintButFail sentinel when it could parse an API error body.
+					// For transport-level errors (no body, e.g. timeouts) it prints
+					// nothing and returns the original error instead, which we must
+					// still surface here.
+					if printErr := cmdx.PrintOpenAPIError(cmd, err); !errors.Is(printErr, cmdx.ErrNoPrintButFail) {
+						_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "%s\n", printErr)
+					}
+					failed = append(failed, a)
 					continue
 				}
 				deleted = append(deleted, cmdx.OutputIder(a))
@@ -61,7 +71,6 @@ func NewDeleteIdentityCmd() *cobra.Command {
 				cmdx.PrintTable(cmd, &cmdx.OutputIderCollection{Items: deleted})
 			}
 
-			cmdx.PrintErrors(cmd, failed)
 			if len(failed) != 0 {
 				return cmdx.FailSilently(cmd)
 			}

cmd/identities/delete_test.go

@@ -6,11 +6,14 @@ package identities_test
 import (
 	"context"
 	"errors"
+	"net/http"
 	"strings"
 	"testing"
 
+	"github.com/spf13/cobra"
 	"github.com/tidwall/gjson"
 
+	"github.com/ory/kratos/cmd/cliclient"
 	"github.com/ory/kratos/cmd/identities"
 
 	"github.com/stretchr/testify/assert"
@@ -19,6 +22,7 @@ import (
 	"github.com/ory/kratos/driver/config"
 	"github.com/ory/kratos/identity"
 	"github.com/ory/kratos/x"
+	"github.com/ory/x/cmdx"
 	"github.com/ory/x/sqlcon"
 )
 
@@ -54,8 +58,79 @@ func TestDeleteCmd(t *testing.T) {
 	})
 
 	t.Run("case=fails with unknown ID", func(t *testing.T) {
-		stdErr := cmd.ExecExpectedErr(t, x.NewUUID().String())
+		unknownID := x.NewUUID().String()
+		stdErr := cmd.ExecExpectedErr(t, unknownID)
 
 		assert.Contains(t, stdErr, "Unable to locate the resource", stdErr)
+		assert.Contains(t, stdErr, unknownID, stdErr)
+		assert.NotContains(t, stdErr, "this error should never be printed", stdErr)
 	})
+
+	t.Run("case=fails with mix of known and unknown IDs", func(t *testing.T) {
+		is, ids := makeIdentities(t, reg, 1)
+		unknownID := x.NewUUID().String()
+
+		// the unknown ID goes first: a regression that returns on the first
+		// error instead of continuing the loop would still delete ids[0] and
+		// pass if it were listed first, so this ordering is what actually
+		// proves deletion continues past a failure.
+		stdOut, stdErr, err := cmd.Exec(nil, unknownID, ids[0])
+		require.Error(t, err)
+
+		assert.Contains(t, stdOut, ids[0], stdOut)
+		assert.NotContains(t, stdOut, unknownID, stdOut)
+		assert.Contains(t, stdErr, unknownID, stdErr)
+		assert.NotContains(t, stdErr, "this error should never be printed", stdErr)
+
+		// the known identity should still have been deleted despite the other ID failing
+		_, err = reg.Persister().GetIdentity(context.Background(), is[0].ID, identity.ExpandNothing)
+		assert.True(t, errors.Is(err, sqlcon.ErrNoRows()))
+	})
+
+	t.Run("case=surfaces transport errors instead of the sentinel", func(t *testing.T) {
+		// PrintOpenAPIError only pretty-prints and returns the ErrNoPrintButFail
+		// sentinel for errors that carry a parsed API response body. A transport
+		// failure has no body, so PrintOpenAPIError returns the original error
+		// unprinted; the command must still surface it instead of discarding it.
+		//
+		// The failure is injected via a RoundTripper returning a known error,
+		// rather than dialing a real port, so the test is hermetic and asserts
+		// on a value we control instead of an OS-dependent dial error string.
+		transportErr := errors.New("simulated transport failure")
+		ctx := context.WithValue(context.Background(), cliclient.ClientContextKey,
+			func(cmd *cobra.Command) (*cliclient.ClientContext, error) {
+				return &cliclient.ClientContext{
+					Endpoint:   "http://localhost",
+					HTTPClient: &http.Client{Transport: failingRoundTripper{err: transportErr}},
+				}, nil
+			})
+
+		unreachable := &cmdx.CommandExecuter{
+			Ctx: ctx,
+			New: func() *cobra.Command {
+				c := identities.NewDeleteIdentityCmd()
+				cliclient.RegisterClientFlags(c.Flags())
+				cmdx.RegisterFormatFlags(c.Flags())
+				return c
+			},
+			PersistentArgs: []string{"--" + cmdx.FlagFormat, string(cmdx.FormatJSON)},
+		}
+
+		stdErr := unreachable.ExecExpectedErr(t, x.NewUUID().String())
+
+		assert.NotContains(t, stdErr, "this error should never be printed", stdErr)
+		// asserts on the actual injected error, not just "something got
+		// printed" - the bug this guards against prints only the "Failed to
+		// delete identity <id>:" header and silently drops the real error,
+		// which would still make a weaker "output is non-empty" check pass.
+		assert.Contains(t, stdErr, transportErr.Error(), stdErr)
+	})
+}
+
+// failingRoundTripper always fails with a known error, standing in for a real
+// transport failure (e.g. a timeout) without depending on real network I/O.
+type failingRoundTripper struct{ err error }
+
+func (rt failingRoundTripper) RoundTrip(*http.Request) (*http.Response, error) {
+	return nil, rt.err
 }