Merge pull request #1954 from freqtrade/fix/stoploss_cancel_error

Trailing stoploss cancel orders should be handled gracefully

freqtrade/freqtradebot.py

@@ -690,13 +690,22 @@ class FreqtradeBot(object):
                 # cancelling the current stoploss on exchange first
                 logger.info('Trailing stoploss: cancelling current stoploss on exchange (id:{%s})'
                             'in order to add another one ...', order['id'])
-                if self.exchange.cancel_order(order['id'], trade.pair):
+                try:
+                    self.exchange.cancel_order(order['id'], trade.pair)
+                except InvalidOrderException:
+                    logger.exception(f"Could not cancel stoploss order {order['id']} "
+                                     f"for pair {trade.pair}")
+
+                try:
                     # creating the new one
                     stoploss_order_id = self.exchange.stoploss_limit(
                         pair=trade.pair, amount=trade.amount,
                         stop_price=trade.stop_loss, rate=trade.stop_loss * 0.99
                     )['id']
                     trade.stoploss_order_id = str(stoploss_order_id)
+                except DependencyException:
+                    logger.exception(f"Could create trailing stoploss order "
+                                     f"for pair {trade.pair}.")
 
     def check_sell(self, trade: Trade, sell_rate: float, buy: bool, sell: bool) -> bool:
         if self.edge:
@@ -842,7 +851,10 @@ class FreqtradeBot(object):
 
         # First cancelling stoploss on exchange ...
         if self.strategy.order_types.get('stoploss_on_exchange') and trade.stoploss_order_id:
+            try:
                 self.exchange.cancel_order(trade.stoploss_order_id, trade.pair)
+            except InvalidOrderException:
+                logger.exception(f"Could not cancel stoploss order {trade.stoploss_order_id}")
 
         # Execute sell and update trade record
         order_id = self.exchange.sell(pair=str(trade.pair),

freqtrade/tests/conftest.py

@@ -110,11 +110,23 @@ def patch_freqtradebot(mocker, config) -> None:
 
 
 def get_patched_freqtradebot(mocker, config) -> FreqtradeBot:
+    """
+    This function patches _init_modules() to not call dependencies
+    :param mocker: a Mocker object to apply patches
+    :param config: Config to pass to the bot
+    :return: FreqtradeBot
+    """
     patch_freqtradebot(mocker, config)
     return FreqtradeBot(config)
 
 
 def get_patched_worker(mocker, config) -> Worker:
+    """
+    This function patches _init_modules() to not call dependencies
+    :param mocker: a Mocker object to apply patches
+    :param config: Config to pass to the bot
+    :return: Worker
+    """
     patch_freqtradebot(mocker, config)
     return Worker(args=None, config=config)
 

freqtrade/tests/test_freqtradebot.py

@@ -19,47 +19,13 @@ from freqtrade.persistence import Trade
 from freqtrade.rpc import RPCMessageType
 from freqtrade.state import State
 from freqtrade.strategy.interface import SellCheckTuple, SellType
-from freqtrade.tests.conftest import (log_has, log_has_re, patch_edge,
+from freqtrade.tests.conftest import (get_patched_freqtradebot,
-                                      patch_exchange, patch_get_signal,
+                                      get_patched_worker, log_has, log_has_re,
-                                      patch_wallet)
+                                      patch_edge, patch_exchange,
+                                      patch_get_signal, patch_wallet)
 from freqtrade.worker import Worker
 
 
-# Functions for recurrent object patching
-def patch_freqtradebot(mocker, config) -> None:
-    """
-    This function patches _init_modules() to not call dependencies
-    :param mocker: a Mocker object to apply patches
-    :param config: Config to pass to the bot
-    :return: None
-    """
-    mocker.patch('freqtrade.freqtradebot.RPCManager', MagicMock())
-    mocker.patch('freqtrade.freqtradebot.persistence.init', MagicMock())
-    patch_exchange(mocker)
-
-
-def get_patched_freqtradebot(mocker, config) -> FreqtradeBot:
-    """
-    This function patches _init_modules() to not call dependencies
-    :param mocker: a Mocker object to apply patches
-    :param config: Config to pass to the bot
-    :return: FreqtradeBot
-    """
-    patch_freqtradebot(mocker, config)
-    return FreqtradeBot(config)
-
-
-def get_patched_worker(mocker, config) -> Worker:
-    """
-    This function patches _init_modules() to not call dependencies
-    :param mocker: a Mocker object to apply patches
-    :param config: Config to pass to the bot
-    :return: Worker
-    """
-    patch_freqtradebot(mocker, config)
-    return Worker(args=None, config=config)
-
-
 def patch_RPCManager(mocker) -> MagicMock:
     """
     This function mock RPC manager to avoid repeating this code in almost every tests
@@ -1176,6 +1142,77 @@ def test_handle_stoploss_on_exchange_trailing(mocker, default_conf, fee, caplog,
                                                 stop_price=0.00002344 * 0.95)
 
 
+def test_handle_stoploss_on_exchange_trailing_error(mocker, default_conf, fee, caplog,
+                                                    markets, limit_buy_order,
+                                                    limit_sell_order) -> None:
+    # When trailing stoploss is set
+    stoploss_limit = MagicMock(return_value={'id': 13434334})
+    patch_exchange(mocker)
+
+    mocker.patch.multiple(
+        'freqtrade.exchange.Exchange',
+        get_ticker=MagicMock(return_value={
+            'bid': 0.00001172,
+            'ask': 0.00001173,
+            'last': 0.00001172
+        }),
+        buy=MagicMock(return_value={'id': limit_buy_order['id']}),
+        sell=MagicMock(return_value={'id': limit_sell_order['id']}),
+        get_fee=fee,
+        markets=PropertyMock(return_value=markets),
+        stoploss_limit=stoploss_limit
+    )
+
+    # enabling TSL
+    default_conf['trailing_stop'] = True
+
+    freqtrade = get_patched_freqtradebot(mocker, default_conf)
+    # enabling stoploss on exchange
+    freqtrade.strategy.order_types['stoploss_on_exchange'] = True
+
+    # setting stoploss
+    freqtrade.strategy.stoploss = -0.05
+
+    # setting stoploss_on_exchange_interval to 60 seconds
+    freqtrade.strategy.order_types['stoploss_on_exchange_interval'] = 60
+    patch_get_signal(freqtrade)
+    freqtrade.create_trade()
+    trade = Trade.query.first()
+    trade.is_open = True
+    trade.open_order_id = None
+    trade.stoploss_order_id = "abcd"
+    trade.stop_loss = 0.2
+    trade.stoploss_last_update = arrow.utcnow().shift(minutes=-601).datetime.replace(tzinfo=None)
+
+    stoploss_order_hanging = {
+        'id': "abcd",
+        'status': 'open',
+        'type': 'stop_loss_limit',
+        'price': 3,
+        'average': 2,
+        'info': {
+            'stopPrice': '0.1'
+        }
+    }
+    mocker.patch('freqtrade.exchange.Exchange.cancel_order', side_effect=InvalidOrderException())
+    mocker.patch('freqtrade.exchange.Exchange.get_order', stoploss_order_hanging)
+    freqtrade.handle_trailing_stoploss_on_exchange(trade, stoploss_order_hanging)
+    assert log_has_re(r"Could not cancel stoploss order abcd for pair ETH/BTC.*",
+                      caplog.record_tuples)
+
+    # Still try to create order
+    assert stoploss_limit.call_count == 1
+
+    # Fail creating stoploss order
+    caplog.clear()
+    cancel_mock = mocker.patch("freqtrade.exchange.Exchange.cancel_order", MagicMock())
+    mocker.patch("freqtrade.exchange.Exchange.stoploss_limit", side_effect=DependencyException())
+    freqtrade.handle_trailing_stoploss_on_exchange(trade, stoploss_order_hanging)
+    assert cancel_mock.call_count == 1
+    assert log_has_re(r"Could create trailing stoploss order for pair ETH/BTC\..*",
+                      caplog.record_tuples)
+
+
 def test_tsl_on_exchange_compatible_with_edge(mocker, edge_conf, fee, caplog,
                                               markets, limit_buy_order, limit_sell_order) -> None:
 
@@ -2108,6 +2145,36 @@ def test_execute_sell_down_stoploss_on_exchange_dry_run(default_conf, ticker, fe
     } == last_msg
 
 
+def test_execute_sell_sloe_cancel_exception(mocker, default_conf, ticker, fee,
+                                            markets, caplog) -> None:
+    freqtrade = get_patched_freqtradebot(mocker, default_conf)
+    mocker.patch('freqtrade.exchange.Exchange.cancel_order', side_effect=InvalidOrderException())
+    sellmock = MagicMock()
+    mocker.patch.multiple(
+        'freqtrade.exchange.Exchange',
+        _load_markets=MagicMock(return_value={}),
+        get_ticker=ticker,
+        get_fee=fee,
+        markets=PropertyMock(return_value=markets),
+        sell=sellmock
+    )
+
+    freqtrade.strategy.order_types['stoploss_on_exchange'] = True
+    patch_get_signal(freqtrade)
+    freqtrade.create_trade()
+
+    trade = Trade.query.first()
+    Trade.session = MagicMock()
+
+    freqtrade.config['dry_run'] = False
+    trade.stoploss_order_id = "abcd"
+
+    freqtrade.execute_sell(trade=trade, limit=1234,
+                           sell_reason=SellType.STOP_LOSS)
+    assert sellmock.call_count == 1
+    assert log_has('Could not cancel stoploss order abcd', caplog.record_tuples)
+
+
 def test_execute_sell_with_stoploss_on_exchange(default_conf,
                                                 ticker, fee, ticker_sell_up,
                                                 markets, mocker) -> None: