Merge pull request #1727 from mishaker/fix_cancel_order

Adding invalid order exception and fix #1726
This commit is contained in:
Misagh 2019-04-06 20:32:44 +02:00 committed by GitHub
commit 4fef9448bf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 110 additions and 45 deletions

View File

@ -212,6 +212,10 @@ The below is the default which is used if this is not configured in either strat
unsure of what you are doing. For more information about how stoploss works please
read [the stoploss documentation](stoploss.md).
!!! Note
In case of stoploss on exchange if the stoploss is cancelled manually then
the bot would recreate one.
### Understand order_time_in_force
The `order_time_in_force` configuration parameter defines the policy by which the order
is executed on the exchange. Three commonly used time in force are:

View File

@ -17,6 +17,14 @@ class OperationalException(BaseException):
"""
class InvalidOrderException(BaseException):
"""
This is returned when the order is not valid. Example:
If stoploss on exchange order is hit, then trying to cancel the order
should return this exception.
"""
class TemporaryError(BaseException):
"""
Temporary network or exchange related error.

View File

@ -13,7 +13,8 @@ import ccxt
import ccxt.async_support as ccxt_async
from pandas import DataFrame
from freqtrade import constants, DependencyException, OperationalException, TemporaryError
from freqtrade import (constants, DependencyException, OperationalException,
TemporaryError, InvalidOrderException)
from freqtrade.data.converter import parse_ticker_dataframe
logger = logging.getLogger(__name__)
@ -607,7 +608,7 @@ class Exchange(object):
try:
return self._api.cancel_order(order_id, pair)
except ccxt.InvalidOrder as e:
raise DependencyException(
raise InvalidOrderException(
f'Could not cancel order. Message: {e}')
except (ccxt.NetworkError, ccxt.ExchangeError) as e:
raise TemporaryError(
@ -623,8 +624,8 @@ class Exchange(object):
try:
return self._api.fetch_order(order_id, pair)
except ccxt.InvalidOrder as e:
raise DependencyException(
f'Could not get order. Message: {e}')
raise InvalidOrderException(
f'Tried to get an invalid order (id: {order_id}). Message: {e}')
except (ccxt.NetworkError, ccxt.ExchangeError) as e:
raise TemporaryError(
f'Could not get order due to {e.__class__.__name__}. Message: {e}')

View File

@ -11,7 +11,7 @@ from typing import Any, Dict, List, Optional, Tuple
import arrow
from requests.exceptions import RequestException
from freqtrade import (DependencyException, OperationalException,
from freqtrade import (DependencyException, OperationalException, InvalidOrderException,
__version__, constants, persistence)
from freqtrade.data.converter import order_book_to_dataframe
from freqtrade.data.dataprovider import DataProvider
@ -590,46 +590,74 @@ class FreqtradeBot(object):
is enabled.
"""
result = False
logger.debug('Handling stoploss on exchange %s ...', trade)
stoploss_order = None
try:
# If trade is open and the buy order is fulfilled but there is no stoploss,
# then we add a stoploss on exchange
if not trade.open_order_id and not trade.stoploss_order_id:
if self.edge:
stoploss = self.edge.stoploss(pair=trade.pair)
else:
stoploss = self.strategy.stoploss
# First we check if there is already a stoploss on exchange
stoploss_order = self.exchange.get_order(trade.stoploss_order_id, trade.pair) \
if trade.stoploss_order_id else None
except InvalidOrderException as exception:
logger.warning('Unable to fetch stoploss order: %s', exception)
stop_price = trade.open_rate * (1 + stoploss)
# If trade open order id does not exist: buy order is fulfilled
buy_order_fulfilled = not trade.open_order_id
# limit price should be less than stop price.
# 0.99 is arbitrary here.
limit_price = stop_price * 0.99
# Limit price threshold: As limit price should always be below price
limit_price_pct = 0.99
# If buy order is fulfilled but there is no stoploss, we add a stoploss on exchange
if (buy_order_fulfilled and not stoploss_order):
if self.edge:
stoploss = self.edge.stoploss(pair=trade.pair)
else:
stoploss = self.strategy.stoploss
stop_price = trade.open_rate * (1 + stoploss)
# limit price should be less than stop price.
limit_price = stop_price * limit_price_pct
try:
stoploss_order_id = self.exchange.stoploss_limit(
pair=trade.pair, amount=trade.amount, stop_price=stop_price, rate=limit_price
)['id']
trade.stoploss_order_id = str(stoploss_order_id)
trade.stoploss_last_update = datetime.now()
return False
# Or the trade open and there is already a stoploss on exchange.
# so we check if it is hit ...
elif trade.stoploss_order_id:
logger.debug('Handling stoploss on exchange %s ...', trade)
order = self.exchange.get_order(trade.stoploss_order_id, trade.pair)
if order['status'] == 'closed':
trade.sell_reason = SellType.STOPLOSS_ON_EXCHANGE.value
trade.update(order)
self.notify_sell(trade)
result = True
elif self.config.get('trailing_stop', False):
# if trailing stoploss is enabled we check if stoploss value has changed
# in which case we cancel stoploss order and put another one with new
# value immediately
self.handle_trailing_stoploss_on_exchange(trade, order)
except DependencyException as exception:
logger.warning('Unable to create stoploss order: %s', exception)
return result
except DependencyException as exception:
logger.warning('Unable to place a stoploss order on exchange: %s', exception)
# If stoploss order is canceled for some reason we add it
if stoploss_order and stoploss_order['status'] == 'canceled':
try:
stoploss_order_id = self.exchange.stoploss_limit(
pair=trade.pair, amount=trade.amount,
stop_price=trade.stop_loss, rate=trade.stop_loss * limit_price_pct
)['id']
trade.stoploss_order_id = str(stoploss_order_id)
return False
except DependencyException as exception:
logger.warning('Stoploss order was cancelled, '
'but unable to recreate one: %s', exception)
# We check if stoploss order is fulfilled
if stoploss_order and stoploss_order['status'] == 'closed':
trade.sell_reason = SellType.STOPLOSS_ON_EXCHANGE.value
trade.update(stoploss_order)
self.notify_sell(trade)
return True
# Finally we check if stoploss on exchange should be moved up because of trailing.
if stoploss_order and self.config.get('trailing_stop', False):
# if trailing stoploss is enabled we check if stoploss value has changed
# in which case we cancel stoploss order and put another one with new
# value immediately
self.handle_trailing_stoploss_on_exchange(trade, stoploss_order)
return False
def handle_trailing_stoploss_on_exchange(self, trade: Trade, order):
"""
@ -645,8 +673,8 @@ class FreqtradeBot(object):
update_beat = self.strategy.order_types.get('stoploss_on_exchange_interval', 60)
if (datetime.utcnow() - trade.stoploss_last_update).total_seconds() > update_beat:
# cancelling the current stoploss on exchange first
logger.info('Trailing stoploss: cancelling current stoploss on exchange '
'in order to add another one ...')
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):
# creating the new one
stoploss_order_id = self.exchange.stoploss_limit(

View File

@ -11,7 +11,8 @@ import ccxt
import pytest
from pandas import DataFrame
from freqtrade import DependencyException, OperationalException, TemporaryError
from freqtrade import (DependencyException, OperationalException,
TemporaryError, InvalidOrderException)
from freqtrade.exchange import Binance, Exchange, Kraken
from freqtrade.exchange.exchange import API_RETRY_COUNT
from freqtrade.resolvers.exchange_resolver import ExchangeResolver
@ -1233,11 +1234,11 @@ def test_cancel_order(default_conf, mocker, exchange_name):
exchange = get_patched_exchange(mocker, default_conf, api_mock, id=exchange_name)
assert exchange.cancel_order(order_id='_', pair='TKN/BTC') == 123
with pytest.raises(DependencyException):
with pytest.raises(InvalidOrderException):
api_mock.cancel_order = MagicMock(side_effect=ccxt.InvalidOrder)
exchange = get_patched_exchange(mocker, default_conf, api_mock, id=exchange_name)
exchange.cancel_order(order_id='_', pair='TKN/BTC')
assert api_mock.cancel_order.call_count == API_RETRY_COUNT + 1
assert api_mock.cancel_order.call_count == 1
ccxt_exceptionhandlers(mocker, default_conf, api_mock, exchange_name,
"cancel_order", "cancel_order",
@ -1260,11 +1261,11 @@ def test_get_order(default_conf, mocker, exchange_name):
exchange = get_patched_exchange(mocker, default_conf, api_mock, id=exchange_name)
assert exchange.get_order('X', 'TKN/BTC') == 456
with pytest.raises(DependencyException):
with pytest.raises(InvalidOrderException):
api_mock.fetch_order = MagicMock(side_effect=ccxt.InvalidOrder)
exchange = get_patched_exchange(mocker, default_conf, api_mock, id=exchange_name)
exchange.get_order(order_id='_', pair='TKN/BTC')
assert api_mock.fetch_order.call_count == API_RETRY_COUNT + 1
assert api_mock.fetch_order.call_count == 1
ccxt_exceptionhandlers(mocker, default_conf, api_mock, exchange_name,
'get_order', 'fetch_order',

View File

@ -12,7 +12,7 @@ import pytest
import requests
from freqtrade import (DependencyException, OperationalException,
TemporaryError, constants)
TemporaryError, InvalidOrderException, constants)
from freqtrade.data.dataprovider import DataProvider
from freqtrade.freqtradebot import FreqtradeBot
from freqtrade.persistence import Trade
@ -1036,7 +1036,21 @@ def test_handle_stoploss_on_exchange(mocker, default_conf, fee, caplog,
assert freqtrade.handle_stoploss_on_exchange(trade) is False
assert trade.stoploss_order_id == 100
# Third case: when stoploss is set and it is hit
# Third case: when stoploss was set but it was canceled for some reason
# should set a stoploss immediately and return False
trade.is_open = True
trade.open_order_id = None
trade.stoploss_order_id = 100
canceled_stoploss_order = MagicMock(return_value={'status': 'canceled'})
mocker.patch('freqtrade.exchange.Exchange.get_order', canceled_stoploss_order)
stoploss_limit.reset_mock()
assert freqtrade.handle_stoploss_on_exchange(trade) is False
assert stoploss_limit.call_count == 1
assert trade.stoploss_order_id == "13434334"
# Fourth case: when stoploss is set and it is hit
# should unset stoploss_order_id and return true
# as a trade actually happened
freqtrade.create_trade()
@ -1063,7 +1077,16 @@ def test_handle_stoploss_on_exchange(mocker, default_conf, fee, caplog,
side_effect=DependencyException()
)
freqtrade.handle_stoploss_on_exchange(trade)
assert log_has('Unable to create stoploss order: ', caplog.record_tuples)
assert log_has('Unable to place a stoploss order on exchange: ', caplog.record_tuples)
# Fifth case: get_order returns InvalidOrder
# It should try to add stoploss order
trade.stoploss_order_id = 100
stoploss_limit.reset_mock()
mocker.patch('freqtrade.exchange.Exchange.get_order', side_effect=InvalidOrderException())
mocker.patch('freqtrade.exchange.Exchange.stoploss_limit', stoploss_limit)
freqtrade.handle_stoploss_on_exchange(trade)
assert stoploss_limit.call_count == 1
def test_handle_stoploss_on_exchange_trailing(mocker, default_conf, fee, caplog,