Error usage refactoring (#53)

data/redisrepo/cancel_orders_for_user.go

@@ -22,7 +22,7 @@ func (r *redisRepository) CancelOrdersForUser(ctx context.Context, userId uuid.U
 
 	if len(orderIdStrs) == 0 {
 		logctx.Warn(ctx, "no orders found for user", logger.String("userId", userId.String()))
-		return nil, models.ErrNoOrdersFound
+		return nil, models.ErrNotFound
 	}
 
 	// Convert string IDs to UUIDs

data/redisrepo/cancel_orders_for_user_test.go

@@ -79,7 +79,7 @@ func TestRedisRepository_CancelAllOrdersForUser(t *testing.T) {
 		assert.ErrorContains(t, err, "failed to get order IDs for user")
 	})
 
-	t.Run("should return `ErrNoOrdersFound` error if no orders are found for the user", func(t *testing.T) {
+	t.Run("should return `ErrNotFound` error if no orders are found for the user", func(t *testing.T) {
 		db, mock := redismock.NewClientMock()
 
 		repo := &redisRepository{
@@ -90,7 +90,7 @@ func TestRedisRepository_CancelAllOrdersForUser(t *testing.T) {
 
 		orderIds, err := repo.CancelOrdersForUser(ctx, mocks.UserId)
 		assert.Empty(t, orderIds)
-		assert.ErrorIs(t, err, models.ErrNoOrdersFound)
+		assert.ErrorIs(t, err, models.ErrNotFound)
 	})
 
 	t.Run("should exit with error if failed to parse order ID", func(t *testing.T) {
@@ -113,7 +113,7 @@ func TestRedisRepository_CancelAllOrdersForUser(t *testing.T) {
 		}
 
 		mock.ExpectZRange(CreateUserOrdersKey(mocks.UserId), 0, -1).SetVal([]string{mocks.OrderId.String(), orderTwo.Id.String()})
-		mock.ExpectHGetAll(CreateOrderIDKey(orderTwo.Id)).SetErr(models.ErrOrderNotFound) // order not found - break out of loop iteration
+		mock.ExpectHGetAll(CreateOrderIDKey(orderTwo.Id)).SetErr(models.ErrNotFound) // order not found - break out of loop iteration
 
 		orderIds, err := repo.CancelOrdersForUser(ctx, mocks.UserId)
 		assert.Empty(t, orderIds)

data/redisrepo/find_order_by_id.go

@@ -21,7 +21,7 @@ func (r *redisRepository) FindOrderById(ctx context.Context, id uuid.UUID, isCli
 		orderIdStr, err := r.client.Get(ctx, CreateClientOIDKey(id)).Result()
 		if err != nil {
 			if err == redis.Nil {
-				return nil, models.ErrOrderNotFound
+				return nil, models.ErrNotFound
 			}
 			logctx.Error(ctx, "could not get order ID by clientOId", logger.Error(err))
 			return nil, err
@@ -44,7 +44,7 @@ func (r *redisRepository) FindOrderById(ctx context.Context, id uuid.UUID, isCli
 	}
 
 	if len(orderMap) == 0 {
-		return nil, models.ErrOrderNotFound
+		return nil, models.ErrNotFound
 	}
 
 	order := &models.Order{}

data/redisrepo/find_order_by_id_test.go

@@ -65,7 +65,7 @@ func TestRedisRepository_FindOrderById(t *testing.T) {
 		mock.ExpectHGetAll(CreateOrderIDKey(orderID)).SetVal(map[string]string{})
 
 		foundOrder, err := repo.FindOrderById(ctx, nonExistentOrderID, false)
-		assert.Error(t, err, models.ErrOrderNotFound.Error())
+		assert.Error(t, err, models.ErrNotFound.Error())
 		assert.Nil(t, foundOrder)
 	})
 
@@ -74,7 +74,7 @@ func TestRedisRepository_FindOrderById(t *testing.T) {
 		mock.ExpectGet(CreateClientOIDKey(nonExistentClientOId)).RedisNil()
 
 		foundOrder, err := repo.FindOrderById(ctx, nonExistentClientOId, true)
-		assert.Error(t, err, models.ErrOrderNotFound.Error())
+		assert.Error(t, err, models.ErrNotFound.Error())
 		assert.Nil(t, foundOrder)
 	})
 

data/redisrepo/get_best_price_for.go

@@ -38,7 +38,7 @@ func (r *redisRepository) GetBestPriceFor(ctx context.Context, symbol models.Sym
 	}
 
 	if len(orderIDs) == 0 {
-		return models.Order{}, models.ErrOrderNotFound
+		return models.Order{}, models.ErrNotFound
 	}
 
 	orderIDStr := orderIDs[0]

data/redisrepo/get_best_price_for_test.go

@@ -63,7 +63,7 @@ func TestRedisRepository_GetBestPriceFor(t *testing.T) {
 		mock.ExpectZRevRange(buyPricesKey, 0, 0).SetVal([]string{})
 		order, err := repo.GetBestPriceFor(ctx, symbol, models.BUY)
 
-		assert.Error(t, err, models.ErrOrderNotFound, "error should be ErrOrderNotFound")
+		assert.Error(t, err, models.ErrNotFound, "error should be ErrNotFound")
 		assert.Equal(t, models.Order{}, order, "should be zero")
 	})
 
@@ -84,7 +84,7 @@ func TestRedisRepository_GetBestPriceFor(t *testing.T) {
 		mock.ExpectZRange(sellPricesKey, 0, 0).SetVal([]string{})
 		order, err := repo.GetBestPriceFor(ctx, symbol, models.SELL)
 
-		assert.Error(t, err, models.ErrOrderNotFound, "error should be ErrOrderNotFound")
+		assert.Error(t, err, models.ErrNotFound, "error should be ErrNotFound")
 		assert.Equal(t, models.Order{}, order, "should be zero")
 	})
 

data/redisrepo/get_user_by_api_key.go

@@ -23,7 +23,7 @@ func (r *redisRepository) GetUserByApiKey(ctx context.Context, apiKey string) (*
 
 	if len(fields) == 0 {
 		logctx.Warn(ctx, "user not found by api key")
-		return nil, models.ErrUserNotFound
+		return nil, models.ErrNotFound
 	}
 
 	userId, err := uuid.Parse(fields["id"])

data/redisrepo/get_user_by_api_key_test.go

@@ -54,7 +54,7 @@ func TestRedisRepository_GetUserByPublicKey(t *testing.T) {
 		user, err := repo.GetUserByApiKey(ctx, mockApiKey)
 
 		assert.Nil(t, user)
-		assert.ErrorIs(t, err, models.ErrUserNotFound)
+		assert.ErrorIs(t, err, models.ErrNotFound)
 	})
 
 	t.Run("should return error on unexpected error getting user by api key", func(t *testing.T) {

data/redisrepo/get_user_by_id.go

@@ -22,7 +22,7 @@ func (r *redisRepository) GetUserById(ctx context.Context, userId uuid.UUID) (*m
 
 	if len(fields) == 0 {
 		logctx.Error(ctx, "user not found by ID", logger.String("userId", userId.String()))
-		return nil, models.ErrUserNotFound
+		return nil, models.ErrNotFound
 	}
 
 	userType, err := models.StrToUserType(fields["type"])

data/redisrepo/get_user_by_id_test.go

@@ -52,7 +52,7 @@ func TestRedisRepository_GetUserById(t *testing.T) {
 		_, err := repo.GetUserById(ctx, mocks.UserId)
 
 		assert.Error(t, err)
-		assert.Equal(t, err, models.ErrUserNotFound)
+		assert.Equal(t, err, models.ErrNotFound)
 	})
 
 	t.Run("should return error on unexpected error getting user by ID", func(t *testing.T) {

data/redisrepo/remove_order.go

@@ -2,6 +2,7 @@ package redisrepo
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/orbs-network/order-book/models"
 	"github.com/orbs-network/order-book/utils/logger"
@@ -43,7 +44,7 @@ func (r *redisRepository) RemoveOrder(ctx context.Context, order models.Order) e
 
 	if err != nil {
 		logctx.Error(ctx, "failed to remove order from Redis", logger.Error(err), logger.String("orderId", order.Id.String()))
-		return models.ErrTransactionFailed
+		return fmt.Errorf("failed to remove order from Redis: %w", err)
 	}
 	// --- END TRANSACTION ---
 

data/redisrepo/remove_order_test.go

@@ -53,7 +53,7 @@ func TestRedisRepository_RemoveOrder(t *testing.T) {
 
 		err := repo.RemoveOrder(ctx, order)
 
-		assert.ErrorIs(t, err, models.ErrTransactionFailed)
+		assert.ErrorContains(t, err, "failed to remove order")
 		assert.NoError(t, mock.ExpectationsWereMet())
 	})
 

models/errors.go

@@ -2,21 +2,18 @@ package models
 
 import "errors"
 
+var ErrNotFound = errors.New("entity not found")
 var ErrOrderAlreadyExists = errors.New("order already exists")
-var ErrOrderNotFound = errors.New("order not found")
-var ErrNoOrdersFound = errors.New("no orders found")
 var ErrUnexpectedError = errors.New("unexpected error")
 var ErrMarshalError = errors.New("marshal error")
 var ErrUserAlreadyExists = errors.New("user already exists")
-var ErrUserNotFound = errors.New("user not found")
 var ErrNoUserInContext = errors.New("no user in context")
 var ErrUnauthorized = errors.New("user not allowed to perform this action")
 var ErrOrderNotOpen = errors.New("order must be status open to perform this action")
-var ErrTransactionFailed = errors.New("transaction failed")
 var ErrInsufficientLiquity = errors.New("not enough liquidity in book to satisfy amountIn")
 var ErrAuctionInvalid = errors.New("orders in the auction can not fill any longer")
 var ErrOrderPending = errors.New("order is pending")
 var ErrInvalidInput = errors.New("invalid input")
 
 // store generic errors
-var ErrValAlreadyInSet = errors.New("the value is already a memeber of the set")
+var ErrValAlreadyInSet = errors.New("the value is already a member of the set")

service/auction.go

@@ -79,7 +79,7 @@ func (s *Service) ConfirmAuction(ctx context.Context, auctionId uuid.UUID) (Conf
 				logctx.Warn(ctx, "Remove Auction", logger.Error(err))
 			}
 
-			return ConfirmAuctionRes{}, models.ErrOrderNotFound
+			return ConfirmAuctionRes{}, models.ErrNotFound
 		} else if !validateOrderFrag(frag, order) {
 			// cancel auction
 			_ = s.orderBookStore.RemoveAuction(ctx, auctionId)
@@ -190,7 +190,7 @@ func (s *Service) AuctionMined(ctx context.Context, auctionId uuid.UUID) error {
 				logctx.Error(ctx, err.Error())
 			}
 			// return empty
-			return models.ErrOrderNotFound
+			return models.ErrNotFound
 		} else if !validatePendingFrag(frag, order) {
 			// cancel auction
 			err = s.orderBookStore.RemoveAuction(ctx, auctionId) // PANIC - shouldn't happen

service/cancel_order.go

@@ -40,7 +40,7 @@ func (s *Service) CancelOrder(ctx context.Context, input CancelOrderInput) (canc
 
 	if order == nil {
 		logctx.Warn(ctx, "order not found", logger.String("id", input.Id.String()), logger.Bool("isClientOId", input.IsClientOId))
-		return nil, models.ErrOrderNotFound
+		return nil, models.ErrNotFound
 	}
 
 	if order.SizePending.GreaterThan(decimal.Zero) {

service/cancel_order_test.go

@@ -32,7 +32,7 @@ func TestService_CancelOrder(t *testing.T) {
 	}{
 		{name: "unexpected error when finding order by orderId - returns error", isClientOId: false, err: assert.AnError, expectedOrderId: nil, expectedErr: assert.AnError},
 		{name: "unexpected error when finding order by clientOId - returns error", isClientOId: true, err: assert.AnError, expectedOrderId: nil, expectedErr: assert.AnError},
-		{name: "order not found - returns `ErrOrderNotFound` error", isClientOId: false, order: nil, err: nil, expectedOrderId: nil, expectedErr: models.ErrOrderNotFound},
+		{name: "order not found - returns `ErrNotFound` error", isClientOId: false, order: nil, err: nil, expectedOrderId: nil, expectedErr: models.ErrNotFound},
 		{name: "user trying to cancel another user's order - returns `ErrUnauthorized` error", isClientOId: false, order: &models.Order{UserId: uuid.MustParse("00000000-0000-0000-0000-000000000009"), Status: models.STATUS_OPEN}, expectedOrderId: nil, expectedErr: models.ErrUnauthorized},
 		{name: "cancelling order not possible when order is pending", isClientOId: false, order: &models.Order{UserId: userId, SizePending: decimal.NewFromFloat(254), SizeFilled: decimal.NewFromFloat(32323.32)}, expectedOrderId: nil, expectedErr: models.ErrOrderPending},
 		{name: "unexpected error when removing order - returns error", isClientOId: false, order: order, err: assert.AnError, expectedOrderId: nil, expectedErr: assert.AnError},

service/cancel_orders_for_user.go

@@ -14,7 +14,7 @@ func (s *Service) CancelOrdersForUser(ctx context.Context, userId uuid.UUID) (or
 
 	orderIds, err = s.orderBookStore.CancelOrdersForUser(ctx, userId)
 
-	if err == models.ErrNoOrdersFound {
+	if err == models.ErrNotFound {
 		logctx.Info(ctx, "no orders found for user", logger.String("userId", userId.String()))
 		return []uuid.UUID{}, err
 	}

service/cancel_orders_for_user_test.go

@@ -25,13 +25,13 @@ func TestService_CancelOrdersForUser(t *testing.T) {
 	})
 
 	t.Run("should return error if no orders found for user", func(t *testing.T) {
-		store := &mocks.MockOrderBookStore{User: &mocks.User, Orders: []models.Order{}, Error: models.ErrNoOrdersFound}
+		store := &mocks.MockOrderBookStore{User: &mocks.User, Orders: []models.Order{}, Error: models.ErrNotFound}
 
 		s, _ := service.New(store, mockBcClient)
 
 		orderIds, err := s.CancelOrdersForUser(ctx, mocks.UserId)
 		assert.Empty(t, orderIds)
-		assert.ErrorIs(t, err, models.ErrNoOrdersFound)
+		assert.ErrorIs(t, err, models.ErrNotFound)
 	})
 
 	t.Run("should return error on unexpected error", func(t *testing.T) {

service/get_best_price_for.go

@@ -12,7 +12,7 @@ import (
 func (s *Service) GetBestPriceFor(ctx context.Context, symbol models.Symbol, side models.Side) (decimal.Decimal, error) {
 	order, err := s.orderBookStore.GetBestPriceFor(ctx, symbol, side)
 
-	if err == models.ErrOrderNotFound {
+	if err == models.ErrNotFound {
 		logctx.Info(ctx, fmt.Sprintf("No orders found for %s %q", side, symbol))
 		return decimal.Zero, nil
 	}

service/get_order_by_client_oid.go

@@ -12,7 +12,7 @@ import (
 func (s *Service) GetOrderByClientOId(ctx context.Context, clientOId uuid.UUID) (*models.Order, error) {
 	order, err := s.orderBookStore.FindOrderById(ctx, clientOId, true)
 
-	if err == models.ErrOrderNotFound {
+	if err == models.ErrNotFound {
 		logctx.Info(ctx, "order not found", logger.String("clientOId", clientOId.String()))
 		return nil, nil
 	}

service/get_order_by_client_oid_test.go

@@ -30,7 +30,7 @@ func TestService_GetOrderByClientOId(t *testing.T) {
 	})
 
 	t.Run("order not found - should return nil", func(t *testing.T) {
-		svc, _ := service.New(&mocks.MockOrderBookStore{Error: models.ErrOrderNotFound}, mockBcClient)
+		svc, _ := service.New(&mocks.MockOrderBookStore{Error: models.ErrNotFound}, mockBcClient)
 
 		order, err := svc.GetOrderByClientOId(ctx, clientOId)
 

service/get_order_by_id.go

@@ -12,7 +12,7 @@ import (
 func (s *Service) GetOrderById(ctx context.Context, orderId uuid.UUID) (*models.Order, error) {
 	order, err := s.orderBookStore.FindOrderById(ctx, orderId, false)
 
-	if err == models.ErrOrderNotFound {
+	if err == models.ErrNotFound {
 		logctx.Info(ctx, "order not found", logger.String("orderId", orderId.String()))
 		return nil, nil
 	}

service/process_order.go

@@ -29,7 +29,7 @@ func (s *Service) ProcessOrder(ctx context.Context, input ProcessOrderInput) (mo
 
 	existingOrder, err := s.orderBookStore.FindOrderById(ctx, input.ClientOrderID, true)
 
-	if err != nil && err != models.ErrOrderNotFound {
+	if err != nil && err != models.ErrNotFound {
 		logctx.Error(ctx, "unexpected error when finding order by clientOrderId", logger.Error(err))
 		return models.Order{}, models.ErrUnexpectedError
 	}

serviceuser/get_user_by_api_key.go

@@ -13,7 +13,7 @@ func (s *Service) GetUserByApiKey(ctx context.Context, apiKey string) (*models.U
 
 	user, err := s.userStore.GetUserByApiKey(ctx, apiKey)
 
-	if err == models.ErrUserNotFound {
+	if err == models.ErrNotFound {
 		logctx.Warn(ctx, "user not found", logger.Error(err))
 		return nil, err
 	}

serviceuser/get_user_by_id_test.go

@@ -21,12 +21,12 @@ func TestServiceUser_GetUserById(t *testing.T) {
 		assert.Equal(t, &mocks.User, user)
 	})
 
-	t.Run("should return `ErrUserNotFound` error if user not found", func(t *testing.T) {
-		userSvc, _ := New(&mocks.MockUserStore{Error: models.ErrUserNotFound})
+	t.Run("should return `ErrNotFound` error if user not found", func(t *testing.T) {
+		userSvc, _ := New(&mocks.MockUserStore{Error: models.ErrNotFound})
 
 		user, err := userSvc.GetUserById(ctx, mocks.UserId)
 
-		assert.ErrorIs(t, err, models.ErrUserNotFound)
+		assert.ErrorIs(t, err, models.ErrNotFound)
 		assert.Nil(t, user)
 	})
 

transport/middleware/validate_user.go

@@ -29,7 +29,7 @@ func ValidateUserMiddleware(getUserByApiKey GetUserByApiKeyFunc) func(http.Handl
 
 			user, err := getUserByApiKey(r.Context(), key)
 			if err != nil {
-				if err == models.ErrUserNotFound {
+				if err == models.ErrNotFound {
 					logctx.Warn(r.Context(), "user not found by api key")
 					http.Error(w, "Unauthorized", http.StatusUnauthorized)
 				} else {

transport/middleware/validate_user_test.go

@@ -38,7 +38,7 @@ func TestValidateUserMiddleware(t *testing.T) {
 		{
 			name:           "should return error if user not found",
 			apiKey:         validApiKey,
-			getUserFunc:    func(ctx context.Context, apiKey string) (*models.User, error) { return nil, models.ErrUserNotFound },
+			getUserFunc:    func(ctx context.Context, apiKey string) (*models.User, error) { return nil, models.ErrNotFound },
 			expectedStatus: http.StatusUnauthorized,
 			expectedBody:   "Unauthorized\n",
 		},

transport/rest/cancel_order.go

@@ -93,7 +93,7 @@ func (h *Handler) handleCancelOrder(input hInput) {
 		UserId:      input.userId,
 	})
 
-	if err == models.ErrOrderNotFound {
+	if err == models.ErrNotFound {
 		logctx.Warn(input.ctx, "order not found", logger.String("id", input.id.String()))
 		http.Error(input.w, "Order not found", http.StatusNotFound)
 		return

transport/rest/cancel_order_test.go

@@ -54,7 +54,7 @@ func TestHandler_CancelOrderByOrderId(t *testing.T) {
 		},
 		{
 			"order not found",
-			&mocks.MockOrderBookService{Error: models.ErrOrderNotFound},
+			&mocks.MockOrderBookService{Error: models.ErrNotFound},
 			fmt.Sprintf("/order/%s", orderId.String()),
 			http.StatusNotFound,
 			"Order not found\n",
@@ -165,7 +165,7 @@ func TestHandler_CancelOrderByClientOId(t *testing.T) {
 		},
 		{
 			"order not found",
-			&mocks.MockOrderBookService{Error: models.ErrOrderNotFound},
+			&mocks.MockOrderBookService{Error: models.ErrNotFound},
 			fmt.Sprintf("/order/client-order/%s", clientOId.String()),
 			http.StatusNotFound,
 			"Order not found\n",

transport/rest/cancel_orders_for_user.go

@@ -27,7 +27,7 @@ func (h *Handler) CancelOrdersForUser(w http.ResponseWriter, r *http.Request) {
 	logctx.Info(ctx, "user trying to cancel all their orders", logger.String("userId", user.Id.String()))
 	orderIds, err := h.svc.CancelOrdersForUser(ctx, user.Id)
 
-	if err == models.ErrNoOrdersFound {
+	if err == models.ErrNotFound {
 		logctx.Info(ctx, "no orders found for user", logger.String("userId", user.Id.String()))
 		http.Error(w, "No orders found", http.StatusNotFound)
 		return

transport/rest/cancel_orders_for_user_test.go

@@ -45,7 +45,7 @@ func TestHandler_CancelOrdersForUser(t *testing.T) {
 	}{
 		{
 			"no orders found for user",
-			&mocks.MockOrderBookService{Error: models.ErrNoOrdersFound},
+			&mocks.MockOrderBookService{Error: models.ErrNotFound},
 			http.StatusNotFound,
 			"No orders found\n",
 		},