diff --git a/pkg/api/handler_balances_list.go b/pkg/api/handler_balances_list.go index f4d4236..64eae59 100644 --- a/pkg/api/handler_balances_list.go +++ b/pkg/api/handler_balances_list.go @@ -1,6 +1,7 @@ package api import ( + "errors" "net/http" wallet "github.com/formancehq/wallets/pkg" @@ -17,6 +18,10 @@ func (m *MainHandler) listBalancesHandler(w http.ResponseWriter, r *http.Request holds, err := m.manager.ListBalances(r.Context(), query) if err != nil { + if errors.Is(err, wallet.ErrValidation) { + badRequest(w, ErrorCodeValidation, err) + return + } internalError(w, r, err) return } diff --git a/pkg/api/handler_holds_get.go b/pkg/api/handler_holds_get.go index 0e26b6a..f7e9549 100644 --- a/pkg/api/handler_holds_get.go +++ b/pkg/api/handler_holds_get.go @@ -1,15 +1,22 @@ package api import ( + "errors" "net/http" + wallet "github.com/formancehq/wallets/pkg" "github.com/go-chi/chi/v5" ) func (m *MainHandler) getHoldHandler(w http.ResponseWriter, r *http.Request) { hold, err := m.manager.GetHold(r.Context(), chi.URLParam(r, "holdID")) if err != nil { - internalError(w, r, err) + switch { + case errors.Is(err, wallet.ErrHoldNotFound): + notFound(w) + default: + internalError(w, r, err) + } return } diff --git a/pkg/api/handler_holds_get_test.go b/pkg/api/handler_holds_get_test.go index 52027fe..553b5d8 100644 --- a/pkg/api/handler_holds_get_test.go +++ b/pkg/api/handler_holds_get_test.go @@ -59,3 +59,38 @@ func TestHoldsGet(t *testing.T) { Remaining: big.NewInt(50), }, ret) } + +func TestHoldsGetNotFound(t *testing.T) { + t.Parallel() + + req := newRequest(t, http.MethodGet, "/holds/"+uuid.NewString(), nil) + rec := httptest.NewRecorder() + + testEnv := newTestEnv( + WithGetAccount(func(ctx context.Context, ledger, account string) (*wallet.AccountWithVolumesAndBalances, error) { + return nil, wallet.ErrAccountNotFound + }), + ) + testEnv.Router().ServeHTTP(rec, req) + + require.Equal(t, http.StatusNotFound, rec.Result().StatusCode) +} + +func TestHoldsGetWrongAccountType(t *testing.T) { + t.Parallel() + + // A non-hold account (no hold metadata) must not be returned as a hold. + req := newRequest(t, http.MethodGet, "/holds/"+uuid.NewString(), nil) + rec := httptest.NewRecorder() + + testEnv := newTestEnv( + WithGetAccount(func(ctx context.Context, ledger, account string) (*wallet.AccountWithVolumesAndBalances, error) { + return &wallet.AccountWithVolumesAndBalances{ + Account: wallet.Account{Metadata: map[string]string{}}, + }, nil + }), + ) + testEnv.Router().ServeHTTP(rec, req) + + require.Equal(t, http.StatusNotFound, rec.Result().StatusCode) +} diff --git a/pkg/api/handler_holds_list.go b/pkg/api/handler_holds_list.go index 30d9286..d9bd466 100644 --- a/pkg/api/handler_holds_list.go +++ b/pkg/api/handler_holds_list.go @@ -1,6 +1,7 @@ package api import ( + "errors" "net/http" wallet "github.com/formancehq/wallets/pkg" @@ -16,6 +17,10 @@ func (m *MainHandler) listHoldsHandler(w http.ResponseWriter, r *http.Request) { holds, err := m.manager.ListHolds(r.Context(), query) if err != nil { + if errors.Is(err, wallet.ErrValidation) { + badRequest(w, ErrorCodeValidation, err) + return + } internalError(w, r, err) return } diff --git a/pkg/api/handler_holds_void.go b/pkg/api/handler_holds_void.go index a205099..16e0f97 100644 --- a/pkg/api/handler_holds_void.go +++ b/pkg/api/handler_holds_void.go @@ -17,6 +17,8 @@ func (m *MainHandler) voidHoldHandler(w http.ResponseWriter, r *http.Request) { }) if err != nil { switch { + case errors.Is(err, wallet.ErrHoldNotFound): + notFound(w) case errors.Is(err, wallet.ErrClosedHold): badRequest(w, ErrorCodeClosedHold, err) default: diff --git a/pkg/api/handler_transactions_list.go b/pkg/api/handler_transactions_list.go index c9a6318..e883f89 100644 --- a/pkg/api/handler_transactions_list.go +++ b/pkg/api/handler_transactions_list.go @@ -1,6 +1,7 @@ package api import ( + "errors" "net/http" wallet "github.com/formancehq/wallets/pkg" @@ -14,6 +15,10 @@ func (m *MainHandler) listTransactions(w http.ResponseWriter, r *http.Request) { }) transactions, err := m.manager.ListTransactions(r.Context(), query) if err != nil { + if errors.Is(err, wallet.ErrValidation) { + badRequest(w, ErrorCodeValidation, err) + return + } internalError(w, r, err) return } diff --git a/pkg/api/handler_wallets_debit_test.go b/pkg/api/handler_wallets_debit_test.go index 4728941..5abaf76 100644 --- a/pkg/api/handler_wallets_debit_test.go +++ b/pkg/api/handler_wallets_debit_test.go @@ -40,7 +40,7 @@ func compareJSON(t *testing.T, expected, actual any) { type testCase struct { name string request wallet.DebitRequest - postTransactionError *sdkerrors.WalletsErrorResponse + postTransactionError error expectedPostTransaction func(testEnv *testEnv, walletID string, h *wallet.DebitHold) wallet.PostTransaction expectedStatusCode int expectedErrorCode string @@ -129,8 +129,20 @@ var walletDebitTestCases = []testCase{ request: wallet.DebitRequest{ Amount: wallet.NewMonetary(big.NewInt(100), "USD"), }, - postTransactionError: &sdkerrors.WalletsErrorResponse{ - ErrorCode: sdkerrors.SchemasWalletsErrorResponseErrorCodeInsufficientFund, + // The Ledger interface (DefaultLedger) translates the SDK's + // *sdkerrors.V2ErrorResponse into wallet.ErrInsufficientFundError, + // so the mock returns that domain error directly. + postTransactionError: wallet.ErrInsufficientFundError, + expectedStatusCode: http.StatusBadRequest, + expectedErrorCode: string(shared.ErrorsEnumInsufficientFund), + }, + { + // Every resolved balance is expired, so the source set is empty. + // This must surface as INSUFFICIENT_FUND, not a ledger compile 500. + name: "with only expired balance", + request: wallet.DebitRequest{ + Amount: wallet.NewMonetary(big.NewInt(100), "USD"), + Balances: []string{"coupon3"}, }, expectedStatusCode: http.StatusBadRequest, expectedErrorCode: string(shared.ErrorsEnumInsufficientFund), diff --git a/pkg/api/handler_wallets_list.go b/pkg/api/handler_wallets_list.go index 6eee169..3693be7 100644 --- a/pkg/api/handler_wallets_list.go +++ b/pkg/api/handler_wallets_list.go @@ -1,6 +1,7 @@ package api import ( + "errors" "net/http" wallet "github.com/formancehq/wallets/pkg" @@ -16,6 +17,10 @@ func (m *MainHandler) listWalletsHandler(w http.ResponseWriter, r *http.Request) }) response, err := m.manager.ListWallets(r.Context(), query) if err != nil { + if errors.Is(err, wallet.ErrValidation) { + badRequest(w, ErrorCodeValidation, err) + return + } internalError(w, r, err) return } diff --git a/pkg/api/utils.go b/pkg/api/utils.go index 6b4f154..8f83881 100644 --- a/pkg/api/utils.go +++ b/pkg/api/utils.go @@ -35,12 +35,14 @@ func badRequest(w http.ResponseWriter, code string, err error) { } func internalError(w http.ResponseWriter, r *http.Request, err error) { + // Log the full error server-side, but never echo it to the client: + // wrapped errors leak ledger URLs, account addresses and SDK internals. sharedlogging.FromContext(r.Context()).Error(err) w.WriteHeader(http.StatusInternalServerError) if err := json.NewEncoder(w).Encode(sharedapi.ErrorResponse{ ErrorCode: "INTERNAL_ERROR", - ErrorMessage: err.Error(), + ErrorMessage: "internal server error", }); err != nil { panic(err) } diff --git a/pkg/error.go b/pkg/error.go index 8003578..0a658bb 100644 --- a/pkg/error.go +++ b/pkg/error.go @@ -17,6 +17,7 @@ var ( ErrBalanceNotExists = errors.New("balance not exists") ErrInvalidBalanceSpecified = errors.New("invalid balance specified") ErrNegativeAmount = errors.New("negative amount provided") + ErrValidation = errors.New("validation error") ) type GenericOpenAPIError interface { diff --git a/pkg/ledger_interface.go b/pkg/ledger_interface.go index d3d467a..87a5de8 100644 --- a/pkg/ledger_interface.go +++ b/pkg/ledger_interface.go @@ -174,12 +174,25 @@ func (d DefaultLedger) ListTransactions(ctx context.Context, ledger string, q Li rsp, err := d.client.Ledger.V2.ListTransactions(ctx, req) if err != nil { - return nil, err + return nil, mapListError(err) } return &rsp.V2TransactionsCursorResponse.Cursor, nil } +// mapListError translates a ledger validation error (e.g. a malformed +// pagination cursor) into ErrValidation so handlers can answer 400 instead +// of leaking a 500. +func mapListError(err error) error { + switch v := err.(type) { + case *sdkerrors.V2ErrorResponse: + if v.ErrorCode == shared.V2ErrorsEnumValidation { + return errors.Wrap(ErrValidation, err.Error()) + } + } + return err +} + func (d DefaultLedger) CreateTransaction(ctx context.Context, ledger, ik string, transaction PostTransaction) (*shared.V2Transaction, error) { ret, err := d.client.Ledger.V2.CreateTransaction(ctx, operations.V2CreateTransactionRequest{ V2PostTransaction: shared.V2PostTransaction{ @@ -198,6 +211,12 @@ func (d DefaultLedger) CreateTransaction(ctx context.Context, ledger, ik string, IdempotencyKey: pointer.For(ik), }) if err != nil { + switch v := err.(type) { + case *sdkerrors.V2ErrorResponse: + if v.ErrorCode == shared.V2ErrorsEnumInsufficientFund { + return nil, errors.Wrap(ErrInsufficientFundError, err.Error()) + } + } return nil, err } @@ -285,7 +304,7 @@ func (d DefaultLedger) ListAccounts(ctx context.Context, ledger string, q ListAc ret, err := d.client.Ledger.V2.ListAccounts(ctx, req) if err != nil { - return nil, err + return nil, mapListError(err) } return &AccountsCursorResponseCursor{ diff --git a/pkg/manager.go b/pkg/manager.go index c9566d6..d0c1bac 100644 --- a/pkg/manager.go +++ b/pkg/manager.go @@ -10,8 +10,6 @@ import ( "github.com/formancehq/go-libs/v5/pkg/types/pointer" "github.com/formancehq/go-libs/v5/pkg/types/time" - "github.com/formancehq/formance-sdk-go/v3/pkg/models/sdkerrors" - "github.com/formancehq/formance-sdk-go/v3/pkg/models/shared" "github.com/formancehq/go-libs/v5/pkg/types/metadata" "github.com/pkg/errors" @@ -172,6 +170,13 @@ func (m *Manager) Debit(ctx context.Context, ik string, debit Debit) (*DebitHold sources = append(sources, m.chart.GetBalanceAccount(debit.WalletID, balance.Name)) } + // All resolved balances are expired: there is nothing to debit from. + // Return a domain error rather than building a script with an empty + // source set, which the ledger would reject as a compile error (500). + if len(sources) == 0 { + return nil, ErrInsufficientFundError + } + postTransaction := PostTransaction{ Script: &shared.V2PostTransactionScript{ Plain: pointer.For(BuildDebitWalletScript(metadata, sources...)), @@ -200,6 +205,9 @@ func (m *Manager) Debit(ctx context.Context, ik string, debit Debit) (*DebitHold func (m *Manager) ConfirmHold(ctx context.Context, ik string, debit ConfirmHold) error { account, err := m.client.GetAccount(ctx, m.ledgerName, m.chart.GetHoldAccount(debit.HoldID)) if err != nil { + if errors.Is(err, ErrAccountNotFound) { + return ErrHoldNotFound + } return errors.Wrap(err, "getting account") } if !IsHold(account) { @@ -248,14 +256,23 @@ func (m *Manager) VoidHold(ctx context.Context, ik string, void VoidHold) error if err != nil { return fmt.Errorf("retrieving original transaction: %w", err) } + if len(txs.Data) == 0 { + return ErrHoldNotFound + } if len(txs.Data) != 1 { return fmt.Errorf("expected 1 transaction, got %d", len(txs.Data)) } account, err := m.client.GetAccount(ctx, m.ledgerName, m.chart.GetHoldAccount(void.HoldID)) if err != nil { + if errors.Is(err, ErrAccountNotFound) { + return ErrHoldNotFound + } return errors.Wrap(err, "getting account") } + if !IsHold(account) { + return ErrHoldNotFound + } hold := ExpandedDebitHoldFromLedgerAccount(*account) if hold.IsClosed() { @@ -314,13 +331,11 @@ func (m *Manager) Credit(ctx context.Context, ik string, credit Credit) error { func (m *Manager) CreateTransaction(ctx context.Context, ik string, postTransaction PostTransaction) error { if _, err := m.client.CreateTransaction(ctx, m.ledgerName, ik, postTransaction); err != nil { - switch err := err.(type) { - case *sdkerrors.WalletsErrorResponse: - if err.ErrorCode == sdkerrors.SchemasWalletsErrorResponseErrorCodeInsufficientFund { - return ErrInsufficientFundError - } + // The ledger SDK returns a *sdkerrors.V2ErrorResponse, which the + // ledger layer already translates to ErrInsufficientFundError. + if errors.Is(err, ErrInsufficientFundError) { + return ErrInsufficientFundError } - return errors.Wrap(err, "creating transaction") } @@ -465,6 +480,9 @@ func (m *Manager) GetWallet(ctx context.Context, id string) (*Wallet, error) { m.chart.GetMainBalanceAccount(id), ) if err != nil { + if errors.Is(err, ErrAccountNotFound) { + return nil, ErrWalletNotFound + } return nil, errors.Wrap(err, "getting account") } @@ -549,9 +567,16 @@ func (m *Manager) GetWalletSummary(ctx context.Context, id string) (*Summary, er func (m *Manager) GetHold(ctx context.Context, id string) (*ExpandedDebitHold, error) { account, err := m.client.GetAccount(ctx, m.ledgerName, m.chart.GetHoldAccount(id)) if err != nil { + if errors.Is(err, ErrAccountNotFound) { + return nil, ErrHoldNotFound + } return nil, err } + if !IsHold(account) { + return nil, ErrHoldNotFound + } + return Ptr(ExpandedDebitHoldFromLedgerAccount(*account)), nil } @@ -590,6 +615,9 @@ func (m *Manager) CreateBalance(ctx context.Context, data *CreateBalance) (*Bala func (m *Manager) GetBalance(ctx context.Context, walletID string, balanceName string) (*ExpandedBalance, error) { account, err := m.client.GetAccount(ctx, m.ledgerName, m.chart.GetBalanceAccount(walletID, balanceName)) if err != nil { + if errors.Is(err, ErrAccountNotFound) { + return nil, ErrBalanceNotExists + } return nil, err }