diff --git a/internal/cli/app.go b/internal/cli/app.go index 943286c..406220d 100644 --- a/internal/cli/app.go +++ b/internal/cli/app.go @@ -89,14 +89,34 @@ func mapAppError(err error) error { switch { case errors.Is(err, kwallet.ErrKWalletUnavailable): - return fmt.Errorf("kwallet is not available; make sure KDE Wallet is installed and your session D-Bus is running") + return newUserFacingError("kwallet is not available; make sure KDE Wallet is installed and your session D-Bus is running", err) case errors.Is(err, kwallet.ErrKWalletDisabled): - return fmt.Errorf("kwallet is disabled in this KDE session") + return newUserFacingError("kwallet is disabled in this KDE session", err) case errors.Is(err, kwallet.ErrKWalletOpenFailed): - return fmt.Errorf("kwallet could not be opened; unlock the wallet and try again") + return newUserFacingError("kwallet could not be opened; unlock the wallet and try again", err) case errors.Is(err, kwallet.ErrCredentialNotFound): - return fmt.Errorf("credentials not configured; run `email-mcp setup`") + return newUserFacingError("credentials not configured; run `email-mcp setup`", err) default: return err } } + +type userFacingError struct { + message string + err error +} + +func (e *userFacingError) Error() string { + return e.message +} + +func (e *userFacingError) Unwrap() error { + return e.err +} + +func newUserFacingError(message string, err error) error { + return &userFacingError{ + message: message, + err: err, + } +} diff --git a/internal/cli/app_task6_test.go b/internal/cli/app_task6_test.go index 79ec139..4a3c452 100644 --- a/internal/cli/app_task6_test.go +++ b/internal/cli/app_task6_test.go @@ -52,6 +52,19 @@ func TestMapAppErrorMapsMissingCredentialError(t *testing.T) { if !strings.Contains(err.Error(), "run `email-mcp setup`") { t.Fatalf("expected setup guidance, got %v", err) } + if !errors.Is(err, kwallet.ErrCredentialNotFound) { + t.Fatalf("expected typed error to be preserved, got %v", err) + } +} + +func TestMapAppErrorPreservesUnavailableTypedError(t *testing.T) { + err := mapAppError(fmt.Errorf("%w: session bus missing", kwallet.ErrKWalletUnavailable)) + if err == nil { + t.Fatal("expected mapped error") + } + if !errors.Is(err, kwallet.ErrKWalletUnavailable) { + t.Fatalf("expected typed error to be preserved, got %v", err) + } } func TestMapAppErrorLeavesUnknownErrorsUntouched(t *testing.T) { diff --git a/internal/secretstore/kwallet/client.go b/internal/secretstore/kwallet/client.go index 4f3aab1..55b7d54 100644 --- a/internal/secretstore/kwallet/client.go +++ b/internal/secretstore/kwallet/client.go @@ -48,6 +48,7 @@ type walletSession struct { services []kwalletService object dbusObject handle int32 + opened bool } type sessionBusConnection struct { @@ -135,7 +136,7 @@ func (s *walletSession) open(ctx context.Context) error { if err != nil { return err } - if s.handle != 0 { + if s.opened { return nil } @@ -167,6 +168,7 @@ func (s *walletSession) open(ctx context.Context) error { } s.handle = handle + s.opened = true return nil } @@ -182,7 +184,7 @@ func (s *walletSession) writeEntry(ctx context.Context, key string, value []byte code, err := s.callInt32(ctx, object, "writeEntry", s.handle, kwalletFolderName, key, value, kwalletAppID) if err != nil { - return fmt.Errorf("kwallet write failed: %w", err) + return wrapUnavailable("kwallet write failed", err) } if code != 0 { return fmt.Errorf("kwallet write failed with code %d", code) @@ -202,7 +204,7 @@ func (s *walletSession) readEntry(ctx context.Context, key string) ([]byte, erro hasEntry, err := s.callBool(ctx, object, "hasEntry", s.handle, kwalletFolderName, key, kwalletAppID) if err != nil { - return nil, err + return nil, wrapUnavailable("kwallet entry lookup failed", err) } if !hasEntry { return nil, fmt.Errorf("%w: key %q", ErrCredentialNotFound, key) @@ -210,7 +212,7 @@ func (s *walletSession) readEntry(ctx context.Context, key string) ([]byte, erro value, err := s.callBytes(ctx, object, "readEntry", s.handle, kwalletFolderName, key, kwalletAppID) if err != nil { - return nil, err + return nil, wrapUnavailable("kwallet read failed", err) } return value, nil } @@ -306,3 +308,26 @@ func (s *walletSession) callBytes(ctx context.Context, object dbusObject, name s func kwalletMethod(name string) string { return kwalletInterface + "." + name } + +type typedError struct { + message string + errs []error +} + +func (e *typedError) Error() string { + return e.message +} + +func (e *typedError) Unwrap() []error { + return e.errs +} + +func wrapUnavailable(message string, err error) error { + if err == nil || errors.Is(err, ErrKWalletUnavailable) { + return err + } + return &typedError{ + message: fmt.Sprintf("%s: %v", message, err), + errs: []error{ErrKWalletUnavailable, err}, + } +} diff --git a/internal/secretstore/kwallet/client_test.go b/internal/secretstore/kwallet/client_test.go index d15989d..e2e846e 100644 --- a/internal/secretstore/kwallet/client_test.go +++ b/internal/secretstore/kwallet/client_test.go @@ -114,6 +114,41 @@ func TestClientOpenUsesNetworkWalletAndCreatesFolder(t *testing.T) { } } +func TestClientOpenDoesNotReopenWhenHandleZeroIsValid(t *testing.T) { + walletObject := &stubObject{ + responses: map[string][]stubCall{ + kwalletMethod("isEnabled"): {{body: []any{true}}}, + kwalletMethod("networkWallet"): {{body: []any{"kdewallet"}}}, + kwalletMethod("open"): {{body: []any{int32(0)}}}, + kwalletMethod("hasFolder"): {{body: []any{true}}}, + }, + } + client := newClientImpl(newWalletSession(func() (dbusConnection, error) { + return &stubConnection{ + objects: map[string]*stubObject{ + "org.kde.kwalletd6|/modules/kwalletd6": walletObject, + }, + }, nil + })) + + if err := client.Open(context.Background()); err != nil { + t.Fatalf("first Open returned error: %v", err) + } + if err := client.Open(context.Background()); err != nil { + t.Fatalf("second Open returned error: %v", err) + } + + openCalls := 0 + for _, call := range walletObject.calls { + if call.method == kwalletMethod("open") { + openCalls++ + } + } + if openCalls != 1 { + t.Fatalf("expected one open call, got %d", openCalls) + } +} + func TestClientWriteEntryWritesBytesToConfiguredFolder(t *testing.T) { walletObject := &stubObject{ responses: map[string][]stubCall{ @@ -202,3 +237,60 @@ func TestClientReadEntryReadsStoredPayload(t *testing.T) { t.Fatalf("unexpected payload: %q", got) } } + +func TestClientWriteEntryMapsTransportFailuresToUnavailable(t *testing.T) { + wantErr := errors.New("transport closed") + walletObject := &stubObject{ + responses: map[string][]stubCall{ + kwalletMethod("isEnabled"): {{body: []any{true}}}, + kwalletMethod("networkWallet"): {{body: []any{"kdewallet"}}}, + kwalletMethod("open"): {{body: []any{int32(42)}}}, + kwalletMethod("hasFolder"): {{body: []any{true}}}, + kwalletMethod("writeEntry"): {{err: wantErr}}, + }, + } + client := newClientImpl(newWalletSession(func() (dbusConnection, error) { + return &stubConnection{ + objects: map[string]*stubObject{ + "org.kde.kwalletd6|/modules/kwalletd6": walletObject, + }, + }, nil + })) + + err := client.WriteEntry(context.Background(), "default", []byte("payload")) + if !errors.Is(err, ErrKWalletUnavailable) { + t.Fatalf("expected unavailable error, got %v", err) + } + if !errors.Is(err, wantErr) { + t.Fatalf("expected wrapped transport error, got %v", err) + } +} + +func TestClientReadEntryMapsTransportFailuresToUnavailable(t *testing.T) { + wantErr := errors.New("transport closed") + walletObject := &stubObject{ + responses: map[string][]stubCall{ + kwalletMethod("isEnabled"): {{body: []any{true}}}, + kwalletMethod("networkWallet"): {{body: []any{"kdewallet"}}}, + kwalletMethod("open"): {{body: []any{int32(42)}}}, + kwalletMethod("hasFolder"): {{body: []any{true}}}, + kwalletMethod("hasEntry"): {{body: []any{true}}}, + kwalletMethod("readEntry"): {{err: wantErr}}, + }, + } + client := newClientImpl(newWalletSession(func() (dbusConnection, error) { + return &stubConnection{ + objects: map[string]*stubObject{ + "org.kde.kwalletd6|/modules/kwalletd6": walletObject, + }, + }, nil + })) + + _, err := client.ReadEntry(context.Background(), "default") + if !errors.Is(err, ErrKWalletUnavailable) { + t.Fatalf("expected unavailable error, got %v", err) + } + if !errors.Is(err, wantErr) { + t.Fatalf("expected wrapped transport error, got %v", err) + } +}