From 6c6f9d8e9612d9240f36c5db6d4cb34c476563d5 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Sat, 25 Jul 2026 12:30:01 -0400 Subject: [PATCH 1/2] refactor(initcmd): delete the failureKind relay Closes #34 --- internal/cmd/initcmd/init.go | 20 +----------- internal/cmd/initcmd/initializer.go | 48 +++++++++-------------------- 2 files changed, 15 insertions(+), 53 deletions(-) diff --git a/internal/cmd/initcmd/init.go b/internal/cmd/initcmd/init.go index d4c0f68..da5a8f5 100644 --- a/internal/cmd/initcmd/init.go +++ b/internal/cmd/initcmd/init.go @@ -135,7 +135,7 @@ func run(command *cobra.Command, dependencies Dependencies, flags Setup, nonInte }, }) if err != nil { - return classifyInitialization(err) + return err } if result.Verified { if _, err := fmt.Fprintf(command.ErrOrStderr(), "Authenticated as %s.\n", result.User.AccountID); err != nil { @@ -148,24 +148,6 @@ func run(command *cobra.Command, dependencies Dependencies, flags Setup, nonInte return nil } -func classifyInitialization(err error) error { - var failure *initializationFailure - if !errors.As(err, &failure) { - return exitcode.New(exitcode.Generic, err) - } - switch failure.kind { - case failureGeneric: - return exitcode.New(exitcode.Generic, failure.err) - case failureConfig: - return exitcode.New(exitcode.Config, failure.err) - case failureAuthorization: - return classifyAuthorization(failure.err) - case failureVerification: - return classifyVerification(failure.err) - } - return exitcode.New(exitcode.Generic, failure.err) -} - func classifyAuthorization(err error) error { switch { case errors.Is(err, auth.ErrInvalidCallback), errors.Is(err, auth.ErrStateMismatch): diff --git a/internal/cmd/initcmd/initializer.go b/internal/cmd/initcmd/initializer.go index dfdd701..fb106ff 100644 --- a/internal/cmd/initcmd/initializer.go +++ b/internal/cmd/initcmd/initializer.go @@ -12,6 +12,7 @@ import ( "github.com/open-cli-collective/spotify-cli/internal/client" "github.com/open-cli-collective/spotify-cli/internal/config" "github.com/open-cli-collective/spotify-cli/internal/credentials" + "github.com/open-cli-collective/spotify-cli/internal/exitcode" "github.com/open-cli-collective/spotify-cli/internal/token" ) @@ -53,74 +54,53 @@ type InitializeResult struct { Verified bool } -type failureKind uint8 - -const ( - failureGeneric failureKind = iota - failureConfig - failureAuthorization - failureVerification -) - -type initializationFailure struct { - kind failureKind - err error -} - -func (failure *initializationFailure) Error() string { return failure.err.Error() } -func (failure *initializationFailure) Unwrap() error { return failure.err } - -func failInitialization(kind failureKind, err error) error { - return &initializationFailure{kind: kind, err: err} -} - // Initialize authorizes, verifies, and atomically persists one Spotify setup. func (initializer Initializer) Initialize(ctx context.Context, options InitializeOptions) (InitializeResult, error) { if initializer.OpenStore == nil { - return InitializeResult{}, failInitialization(failureGeneric, errors.New("credential store opener is unavailable")) + return InitializeResult{}, exitcode.New(exitcode.Generic, errors.New("credential store opener is unavailable")) } if initializer.SaveConfig == nil { - return InitializeResult{}, failInitialization(failureConfig, errors.New("configuration saver is unavailable")) + return InitializeResult{}, exitcode.New(exitcode.Config, errors.New("configuration saver is unavailable")) } store, err := initializer.OpenStore(credentials.OpenRequest{ Config: options.Config, Backend: options.Backend, BackendSet: options.BackendSet, }) if err != nil { - return InitializeResult{}, failInitialization(failureConfig, errors.New("opening credential store failed")) + return InitializeResult{}, exitcode.New(exitcode.Config, errors.New("opening credential store failed")) } defer func() { _ = store.Close() }() present, err := store.Exists(options.Profile, credentials.OAuthTokenKey) if err != nil { - return InitializeResult{}, failInitialization(failureConfig, errors.New("checking existing Spotify authorization failed")) + return InitializeResult{}, exitcode.New(exitcode.Config, errors.New("checking existing Spotify authorization failed")) } var previous string if present { if !options.Overwrite { - return InitializeResult{}, failInitialization(failureGeneric, fmt.Errorf("%w at %s; use --overwrite or sptfy config clear", credstore.ErrExists, options.Config.CredentialRef)) + return InitializeResult{}, exitcode.New(exitcode.Generic, fmt.Errorf("%w at %s; use --overwrite or sptfy config clear", credstore.ErrExists, options.Config.CredentialRef)) } previous, err = store.Get(options.Profile, credentials.OAuthTokenKey) if err != nil { - return InitializeResult{}, failInitialization(failureConfig, errors.New("reading existing Spotify authorization for rollback failed")) + return InitializeResult{}, exitcode.New(exitcode.Config, errors.New("reading existing Spotify authorization for rollback failed")) } } if initializer.Authorize == nil { - return InitializeResult{}, failInitialization(failureGeneric, errors.New("spotify authorizer is unavailable")) + return InitializeResult{}, exitcode.New(exitcode.Generic, errors.New("spotify authorizer is unavailable")) } envelope, err := initializer.Authorize(ctx, options.Authorization) if err != nil { - return InitializeResult{}, failInitialization(failureAuthorization, err) + return InitializeResult{}, classifyAuthorization(err) } result := InitializeResult{Verified: options.Verify} if options.Verify { if initializer.Verify == nil { - return InitializeResult{}, failInitialization(failureGeneric, errors.New("spotify verifier is unavailable")) + return InitializeResult{}, exitcode.New(exitcode.Generic, errors.New("spotify verifier is unavailable")) } result.User, err = initializer.Verify(ctx, options.Config, envelope) if err != nil { - return InitializeResult{}, failInitialization(failureVerification, err) + return InitializeResult{}, classifyVerification(err) } } @@ -130,21 +110,21 @@ func (initializer Initializer) Initialize(ctx context.Context, options Initializ } encoded, err := token.Encode(envelope, now) if err != nil { - return InitializeResult{}, failInitialization(failureConfig, err) + return InitializeResult{}, exitcode.New(exitcode.Config, err) } setOptions := []credstore.SetOpt(nil) if options.Overwrite { setOptions = append(setOptions, credstore.WithOverwrite()) } if err := store.Set(options.Profile, credentials.OAuthTokenKey, string(encoded), setOptions...); err != nil { - return InitializeResult{}, failInitialization(failureConfig, redactStoreError(err, previous, string(encoded), envelope)) + return InitializeResult{}, exitcode.New(exitcode.Config, redactStoreError(err, previous, string(encoded), envelope)) } if err := initializer.SaveConfig(options.Config); err != nil { rollbackErr := rollbackCredential(store, options.Profile, previous, present) if rollbackErr != nil { rollbackErr = redactStoreError(rollbackErr, previous, string(encoded), envelope) } - return InitializeResult{}, failInitialization(failureConfig, errors.Join(errors.New("saving configuration failed"), rollbackErr)) + return InitializeResult{}, exitcode.New(exitcode.Config, errors.Join(errors.New("saving configuration failed"), rollbackErr)) } return result, nil } From 46ab12505fef3dd75663b0211c2d855aab8a5dde Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Sat, 25 Jul 2026 12:35:14 -0400 Subject: [PATCH 2/2] test(initcmd): pin distributed failure exit codes --- internal/cmd/initcmd/init_test.go | 66 ++++++++++++++++++++++++++++--- 1 file changed, 61 insertions(+), 5 deletions(-) diff --git a/internal/cmd/initcmd/init_test.go b/internal/cmd/initcmd/init_test.go index 05da384..06df529 100644 --- a/internal/cmd/initcmd/init_test.go +++ b/internal/cmd/initcmd/init_test.go @@ -254,6 +254,52 @@ func TestInitClassifiesAuthorizationFailures(t *testing.T) { } } +func TestInitializerFailureSitesRetainExitCodes(t *testing.T) { + tests := []struct { + name string + configure func(*initHarness) + args []string + code int + }{ + {name: "nil store opener", configure: func(h *initHarness) { h.openStore = nil }, code: exitcode.Generic}, + {name: "store open", configure: func(h *initHarness) { + h.openStore = func(credentials.OpenRequest) (CredentialStore, error) { return nil, errors.New("open failed") } + }, code: exitcode.Config}, + {name: "exists", configure: func(h *initHarness) { h.store.existsErr = errors.New("exists failed") }, code: exitcode.Config}, + {name: "existing without overwrite", configure: func(h *initHarness) { + h.store.values["default/"+credentials.OAuthTokenKey] = "old-secret" + }, code: exitcode.Generic}, + {name: "read previous", configure: func(h *initHarness) { + h.store.values["default/"+credentials.OAuthTokenKey] = "old-secret" + h.store.getErr = errors.New("get failed") + }, args: []string{"--overwrite"}, code: exitcode.Config}, + {name: "nil authorizer", configure: func(h *initHarness) { h.authorize = nil }, code: exitcode.Generic}, + {name: "nil verifier", configure: func(h *initHarness) { h.verify = nil }, args: []string{}, code: exitcode.Generic}, + {name: "token encode", configure: func(h *initHarness) { + h.authorize = func(context.Context, auth.Request) (token.Envelope, error) { return token.Envelope{}, nil } + }, code: exitcode.Config}, + {name: "verification config", configure: func(h *initHarness) { + h.verify = func(context.Context, config.Config, token.Envelope) (client.User, error) { + return client.User{}, auth.ErrInvalidGrant + } + }, args: []string{}, code: exitcode.Config}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + harness := newInitHarness(t) + test.configure(harness) + args := append([]string{"--client-id", "client-id"}, test.args...) + if test.args == nil { + args = append(args, "--no-verify") + } + err := harness.execute(args...) + if exitcode.Code(err) != test.code { + t.Fatalf("error=%v code=%d want=%d", err, exitcode.Code(err), test.code) + } + }) + } +} + func TestInitRollsBackCredentialWhenConfigSaveFails(t *testing.T) { tests := []struct { name string @@ -344,6 +390,7 @@ type initHarness struct { now time.Time interactive bool backend string + openStore StoreOpener prompt func(*Setup) error authorize func(context.Context, auth.Request) (token.Envelope, error) verify func(context.Context, config.Config, token.Envelope) (client.User, error) @@ -375,6 +422,10 @@ func newInitHarness(t *testing.T) *initHarness { return config.Save(harness.scope, value) } harness.store.onSet = func() { harness.events = append(harness.events, "set") } + harness.openStore = func(request credentials.OpenRequest) (CredentialStore, error) { + harness.requests = append(harness.requests, request) + return harness.store, nil + } return harness } @@ -390,11 +441,8 @@ func (harness *initHarness) execute(args ...string) error { command := New(Dependencies{ Scope: harness.scope, Interactive: harness.interactive, Prompt: harness.prompt, Initializer: Initializer{ - OpenStore: func(request credentials.OpenRequest) (CredentialStore, error) { - harness.requests = append(harness.requests, request) - return harness.store, nil - }, - Now: func() time.Time { return harness.now }, Authorize: harness.authorize, + OpenStore: harness.openStore, + Now: func() time.Time { return harness.now }, Authorize: harness.authorize, Verify: harness.verify, SaveConfig: harness.saveConfig, }, }) @@ -412,11 +460,16 @@ type initStore struct { setErr error setErrors []error deleteErr error + existsErr error + getErr error onSet func() } func (*initStore) Close() error { return nil } func (store *initStore) Get(profile, key string) (string, error) { + if store.getErr != nil { + return "", store.getErr + } value, ok := store.values[profile+"/"+key] if !ok { return "", credstore.ErrNotFound @@ -447,6 +500,9 @@ func (store *initStore) Delete(profile, key string) error { return nil } func (store *initStore) Exists(profile, key string) (bool, error) { + if store.existsErr != nil { + return false, store.existsErr + } _, ok := store.values[profile+"/"+key] return ok, nil }