Fixes after night runv0.5

- Currency pair doesn’t work when fetching orders
- Store error in main to report.
- Cancel orders when closing trade
- Print closed status of trade to repr
- Don’t try to cancel not cancellable orders

3 files changed, 102 insertions, 34 deletions

diff --git a/main.py b/main.py
index 17cf58c..d4bab02 100644
--- a/main.py
+++ b/main.py
@@ -10,6 +10,9 @@ for market_config, user_id in helper.main_fetch_markets(pg_config, args.user):
         user_market = market.Market.from_config(market_config, debug=args.debug)
         helper.main_process_market(user_market, args.action, before=args.before, after=args.after)
     except Exception as e:
-        print("{}: {}".format(e.__class__.__name__, e))
+        try:
+            user_market.report.log_error("main", exception=e)
+        except:
+            print("{}: {}".format(e.__class__.__name__, e))
     finally:
         helper.main_store_report(report_path, user_id, user_market)
diff --git a/portfolio.py b/portfolio.py
index f27e84f..0f2c011 100644
--- a/portfolio.py
+++ b/portfolio.py
@@ -3,7 +3,7 @@ from datetime import datetime, timedelta
 from decimal import Decimal as D, ROUND_DOWN
 from json import JSONDecodeError
 from simplejson.errors import JSONDecodeError as SimpleJSONDecodeError
-from ccxt import ExchangeError, InsufficientFunds, ExchangeNotAvailable, InvalidOrder, OrderNotCached
+from ccxt import ExchangeError, InsufficientFunds, ExchangeNotAvailable, InvalidOrder, OrderNotCached, OrderNotFound
 from retry import retry
 import requests
 
@@ -333,6 +333,8 @@ class Trade:
         return not (self.is_fullfiled or self.closed)
 
     def close(self):
+        for order in self.orders:
+            order.cancel()
         self.closed = True
 
     @property
@@ -467,11 +469,19 @@ class Trade:
                 }
 
     def __repr__(self):
-        return "Trade({} -> {} in {}, {})".format(
+        if self.closed and not self.is_fullfiled:
+            closed = " ❌"
+        elif self.is_fullfiled:
+            closed = " ✔"
+        else:
+            closed = ""
+
+        return "Trade({} -> {} in {}, {}{})".format(
                 self.value_from,
                 self.value_to,
                 self.currency,
-                self.action)
+                self.action,
+                closed)
 
     def print_with_order(self, ind=""):
         self.market.report.print_log("{}{}".format(ind, self))
@@ -600,7 +610,7 @@ class Order:
             self.market.report.log_debug_action("Fetching {}".format(self))
             return
         try:
-            result = self.market.ccxt.fetch_order(self.id, symbol=self.amount.currency)
+            result = self.market.ccxt.fetch_order(self.id)
             self.results.append(result)
             self.status = result["status"]
             # Time at which the order started
@@ -645,8 +655,12 @@ class Order:
             self.market.report.log_debug_action("Mark {} as cancelled".format(self))
             self.status = "canceled"
             return
-        self.market.ccxt.cancel_order(self.id)
-        self.fetch()
+        if self.open and self.id is not None:
+            try:
+                self.market.ccxt.cancel_order(self.id)
+            except OrderNotFound as e: # Closed inbetween
+                self.market.report.log_error("cancel_order", message="Already cancelled order", exception=e)
+            self.fetch()
 
 class Mouvement:
     def __init__(self, currency, base_currency, hash_):
diff --git a/test.py b/test.py
index 0bfa2b7..a45010b 100644
--- a/test.py
+++ b/test.py
@@ -1746,25 +1746,46 @@ class TradeTest(WebMockTestCase):
         trade.orders.append(order_mock1)
         trade.orders.append(order_mock2)
 
-        trade.print_with_order()
+        with mock.patch.object(trade, "filled_amount") as filled:
+            filled.return_value = portfolio.Amount("BTC", "0.1")
 
-        self.m.report.print_log.assert_called()
-        calls = self.m.report.print_log.mock_calls
-        self.assertEqual("Trade(0.50000000 BTC [10.00000000 ETH] -> 1.00000000 BTC in ETH, acquire)", str(calls[0][1][0]))
-        self.assertEqual("\tMock 1", str(calls[1][1][0]))
-        self.assertEqual("\tMock 2", str(calls[2][1][0]))
-        self.assertEqual("\t\tMouvement 1", str(calls[3][1][0]))
-        self.assertEqual("\t\tMouvement 2", str(calls[4][1][0]))
+            trade.print_with_order()
+
+            self.m.report.print_log.assert_called()
+            calls = self.m.report.print_log.mock_calls
+            self.assertEqual("Trade(0.50000000 BTC [10.00000000 ETH] -> 1.00000000 BTC in ETH, acquire)", str(calls[0][1][0]))
+            self.assertEqual("\tMock 1", str(calls[1][1][0]))
+            self.assertEqual("\tMock 2", str(calls[2][1][0]))
+            self.assertEqual("\t\tMouvement 1", str(calls[3][1][0]))
+            self.assertEqual("\t\tMouvement 2", str(calls[4][1][0]))
+
+            self.m.report.print_log.reset_mock()
+
+            filled.return_value = portfolio.Amount("BTC", "0.5")
+            trade.print_with_order()
+            calls = self.m.report.print_log.mock_calls
+            self.assertEqual("Trade(0.50000000 BTC [10.00000000 ETH] -> 1.00000000 BTC in ETH, acquire ✔)", str(calls[0][1][0]))
+
+            self.m.report.print_log.reset_mock()
+
+            filled.return_value = portfolio.Amount("BTC", "0.1")
+            trade.closed = True
+            trade.print_with_order()
+            calls = self.m.report.print_log.mock_calls
+            self.assertEqual("Trade(0.50000000 BTC [10.00000000 ETH] -> 1.00000000 BTC in ETH, acquire ❌)", str(calls[0][1][0]))
 
     def test_close(self):
         value_from = portfolio.Amount("BTC", "0.5")
         value_from.linked_to = portfolio.Amount("ETH", "10.0")
         value_to = portfolio.Amount("BTC", "1.0")
         trade = portfolio.Trade(value_from, value_to, "ETH", self.m)
+        order1 = mock.Mock()
+        trade.orders.append(order1)
 
         trade.close()
 
         self.assertEqual(True, trade.closed)
+        order1.cancel.assert_called_once_with()
 
     def test_pending(self):
         value_from = portfolio.Amount("BTC", "0.5")
@@ -1888,27 +1909,57 @@ class OrderTest(WebMockTestCase):
 
     @mock.patch.object(portfolio.Order, "fetch")
     def test_cancel(self, fetch):
-        self.m.debug = True
-        order = portfolio.Order("buy", portfolio.Amount("ETH", 10),
-                D("0.1"), "BTC", "long", self.m, "trade")
-        order.status = "open"
+        with self.subTest(debug=True):
+            self.m.debug = True
+            order = portfolio.Order("buy", portfolio.Amount("ETH", 10),
+                    D("0.1"), "BTC", "long", self.m, "trade")
+            order.status = "open"
 
-        order.cancel()
-        self.m.ccxt.cancel_order.assert_not_called()
-        self.m.report.log_debug_action.assert_called_once()
-        self.m.report.log_debug_action.reset_mock()
-        self.assertEqual("canceled", order.status)
+            order.cancel()
+            self.m.ccxt.cancel_order.assert_not_called()
+            self.m.report.log_debug_action.assert_called_once()
+            self.m.report.log_debug_action.reset_mock()
+            self.assertEqual("canceled", order.status)
 
-        self.m.debug = False
-        order = portfolio.Order("buy", portfolio.Amount("ETH", 10),
-                D("0.1"), "BTC", "long", self.m, "trade")
-        order.status = "open"
-        order.id = 42
+        with self.subTest(desc="Nominal case"):
+            self.m.debug = False
+            order = portfolio.Order("buy", portfolio.Amount("ETH", 10),
+                    D("0.1"), "BTC", "long", self.m, "trade")
+            order.status = "open"
+            order.id = 42
 
-        order.cancel()
-        self.m.ccxt.cancel_order.assert_called_with(42)
-        fetch.assert_called_once_with()
-        self.m.report.log_debug_action.assert_not_called()
+            order.cancel()
+            self.m.ccxt.cancel_order.assert_called_with(42)
+            fetch.assert_called_once_with()
+            self.m.report.log_debug_action.assert_not_called()
+
+        with self.subTest(exception=True):
+            self.m.ccxt.cancel_order.side_effect = portfolio.OrderNotFound
+            order = portfolio.Order("buy", portfolio.Amount("ETH", 10),
+                    D("0.1"), "BTC", "long", self.m, "trade")
+            order.status = "open"
+            order.id = 42
+            order.cancel()
+            self.m.ccxt.cancel_order.assert_called_with(42)
+            self.m.report.log_error.assert_called_once()
+
+        self.m.reset_mock()
+        with self.subTest(id=None):
+            self.m.ccxt.cancel_order.side_effect = portfolio.OrderNotFound
+            order = portfolio.Order("buy", portfolio.Amount("ETH", 10),
+                    D("0.1"), "BTC", "long", self.m, "trade")
+            order.status = "open"
+            order.cancel()
+            self.m.ccxt.cancel_order.assert_not_called()
+
+        self.m.reset_mock()
+        with self.subTest(open=False):
+            self.m.ccxt.cancel_order.side_effect = portfolio.OrderNotFound
+            order = portfolio.Order("buy", portfolio.Amount("ETH", 10),
+                    D("0.1"), "BTC", "long", self.m, "trade")
+            order.status = "closed"
+            order.cancel()
+            self.m.ccxt.cancel_order.assert_not_called()
 
     def test_dust_amount_remaining(self):
         order = portfolio.Order("buy", portfolio.Amount("ETH", 10),
@@ -2050,7 +2101,7 @@ class OrderTest(WebMockTestCase):
                     }
             order.fetch()
 
-            self.m.ccxt.fetch_order.assert_called_once_with(45, symbol="ETH")
+            self.m.ccxt.fetch_order.assert_called_once_with(45)
             fetch_mouvements.assert_called_once()
             self.assertEqual("foo", order.status)
             self.assertEqual("timestamp", order.timestamp)