fix: harden kwallet task 6 error handling
This commit is contained in:
parent
14191cae1a
commit
2d97306ea0
4 changed files with 158 additions and 8 deletions
|
|
@ -89,14 +89,34 @@ func mapAppError(err error) error {
|
||||||
|
|
||||||
switch {
|
switch {
|
||||||
case errors.Is(err, kwallet.ErrKWalletUnavailable):
|
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):
|
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):
|
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):
|
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:
|
default:
|
||||||
return err
|
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,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -52,6 +52,19 @@ func TestMapAppErrorMapsMissingCredentialError(t *testing.T) {
|
||||||
if !strings.Contains(err.Error(), "run `email-mcp setup`") {
|
if !strings.Contains(err.Error(), "run `email-mcp setup`") {
|
||||||
t.Fatalf("expected setup guidance, got %v", err)
|
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) {
|
func TestMapAppErrorLeavesUnknownErrorsUntouched(t *testing.T) {
|
||||||
|
|
|
||||||
|
|
@ -48,6 +48,7 @@ type walletSession struct {
|
||||||
services []kwalletService
|
services []kwalletService
|
||||||
object dbusObject
|
object dbusObject
|
||||||
handle int32
|
handle int32
|
||||||
|
opened bool
|
||||||
}
|
}
|
||||||
|
|
||||||
type sessionBusConnection struct {
|
type sessionBusConnection struct {
|
||||||
|
|
@ -135,7 +136,7 @@ func (s *walletSession) open(ctx context.Context) error {
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
if s.handle != 0 {
|
if s.opened {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -167,6 +168,7 @@ func (s *walletSession) open(ctx context.Context) error {
|
||||||
}
|
}
|
||||||
|
|
||||||
s.handle = handle
|
s.handle = handle
|
||||||
|
s.opened = true
|
||||||
return nil
|
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)
|
code, err := s.callInt32(ctx, object, "writeEntry", s.handle, kwalletFolderName, key, value, kwalletAppID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("kwallet write failed: %w", err)
|
return wrapUnavailable("kwallet write failed", err)
|
||||||
}
|
}
|
||||||
if code != 0 {
|
if code != 0 {
|
||||||
return fmt.Errorf("kwallet write failed with code %d", code)
|
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)
|
hasEntry, err := s.callBool(ctx, object, "hasEntry", s.handle, kwalletFolderName, key, kwalletAppID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, wrapUnavailable("kwallet entry lookup failed", err)
|
||||||
}
|
}
|
||||||
if !hasEntry {
|
if !hasEntry {
|
||||||
return nil, fmt.Errorf("%w: key %q", ErrCredentialNotFound, key)
|
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)
|
value, err := s.callBytes(ctx, object, "readEntry", s.handle, kwalletFolderName, key, kwalletAppID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, wrapUnavailable("kwallet read failed", err)
|
||||||
}
|
}
|
||||||
return value, nil
|
return value, nil
|
||||||
}
|
}
|
||||||
|
|
@ -306,3 +308,26 @@ func (s *walletSession) callBytes(ctx context.Context, object dbusObject, name s
|
||||||
func kwalletMethod(name string) string {
|
func kwalletMethod(name string) string {
|
||||||
return kwalletInterface + "." + name
|
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},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -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) {
|
func TestClientWriteEntryWritesBytesToConfiguredFolder(t *testing.T) {
|
||||||
walletObject := &stubObject{
|
walletObject := &stubObject{
|
||||||
responses: map[string][]stubCall{
|
responses: map[string][]stubCall{
|
||||||
|
|
@ -202,3 +237,60 @@ func TestClientReadEntryReadsStoredPayload(t *testing.T) {
|
||||||
t.Fatalf("unexpected payload: %q", got)
|
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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue