Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NaN returned during exchange maintenance #3221

Open
sanket-k opened this issue Apr 26, 2020 · 14 comments
Open

NaN returned during exchange maintenance #3221

sanket-k opened this issue Apr 26, 2020 · 14 comments
Labels
Enhancement Enhancements to the bot. Get lower priority than bugs by default. Good first issue Good issues to get familiar and start contributing to freqtrade RPC Telegram, ReST API, external control, ...

Comments

@sanket-k
Copy link
Contributor

Step 1: Have you search for this issue before posting it?

yes

If you have discovered a bug in the bot, please search our issue tracker.
If it hasn't been reported, please create a new issue.

Step 2: Describe your environment

  • platform: Docker - latest build(Digest:sha256:3e8fe20bf6aaad9095e1ba0924ce2af3368cc55d1fe41eebb546a73a124fb77f)
  • Operating system: ____
  • Python Version: _____ (python -V)
  • CCXT version: _____ (pip freeze | grep ccxt)
  • Branch: Master | Develop
  • Last Commit ID: _____ (git log --format="%H" -n 1)

Step 3: Describe the problem:

the exchange binance was under maintenance from 2020/04/25 02:00 AM (UTC) - 06:00 (UTC) - https://binance.zendesk.com/hc/en-us/articles/360042668671-Binance-System-Upgrade-Notice-2020-04-25-

observed that the bot returned nan% for profit in /status, and the /performance field was empty.
image

note: Also in the earlier builds this response wasn't present.

If the exchange is down could we return the exchange is under maintenance rather than nan, or previous values??

@hroff-1902 hroff-1902 added the RPC Telegram, ReST API, external control, ... label Apr 26, 2020
@xmatthias
Copy link
Member

how can you determine reliably that the exchange is down?

I don't think that CCXT has a method for that - so the only way to determine this is that calls fail - which can also have different reasons (like your network...) is therefore not reliable.

Unless we have a way to determine that reliably, i don't think this can be solved.

@sanket-k
Copy link
Contributor Author

correct me if I am wrong, i dint seem to face this issue in an earlier build prior to feb ..how was the issue handled then ?

@xmatthias
Copy link
Member

identically...
I assume you didn't press /status table while binance was down ... :) and values do return automatically once the exchange is back up, so to be honest, i don't really see this as a problem.

@hroff-1902
Copy link
Member

the point here, in my opinion, is to print something more user friendly instead of 'technical' "nan%"

@xmatthias
Copy link
Member

well but what's the alternative? assuming "exchange down" while you don't know for sure?

maybe it's not the exchange, just your connection - or this pair that's currently not tradable?

@hroff-1902
Copy link
Member

hroff-1902 commented Apr 26, 2020

"No data" / "N/A" (this does not add more info, though. not verbose enough) / "No data from exchange" (this is verbose, but can be too long for telegram output)

@sanket-k
Copy link
Contributor Author

sanket-k commented Apr 27, 2020

well, luckily I did have an other instance running in parallel with feb build

ed9b989d5f95
docker - digest ->
sha256:8037ec586694ee56dcdb84b957f984ed804350561d89d6945e88070e2d265b9e
and i did try /status and received a response
image
maybe this can be tried during the next maintenance ?

so, i was wondering if i missed something?

also, I would agree with @hroff-1902 maybe a ex.dn response would help rather than nan%

@xmatthias
Copy link
Member

xmatthias commented Apr 27, 2020

but printing ex.dn assumes it's the exchange that has a downtime - which (as mentioned above) is not necessarily the problem.

That's easy to say as you KNOW it was down ...

But all the bot knows is that a network call failed. There's many things that can be the reason for that - the exchange downtime is only one of these - and most often not the most likely.

So unless we have a way to RELIABLY determine that the exchange was down (which has to work for all exchanges!), freqtrade will not suggest that's the case as it cannot be sure, but is only guessing.

Also, the docker build won't really help ... best run /version to get the version of the bot.

@sanket-k
Copy link
Contributor Author

sanket-k commented Apr 28, 2020

Yeah, that is a good point,

so could we use ex.dn for atleast all exchanges which return the response of exchange is under maintenance, i think ccxt supports that with fetchStatus(), atleast that would cover a lot of the users i think.
image

as for the build, not sure it would be very helpful (this was my response)
image

@xmatthias xmatthias added the Good first issue Good issues to get familiar and start contributing to freqtrade label Apr 28, 2020
@xmatthias
Copy link
Member

xmatthias commented Apr 28, 2020

well the change that's most likely related is the change to the price caching for RPC methods.

Now I think it's better to show "nan" than to keep the price around "forever" and report that price during an exchange downtime - as price can do "funny things" right after downtimes sometimes - so it's not really representative of the problem.

Anyway, it's noted as an enhancement to RPC - and I've labeled it with "Good first issue" - as i think the complexity of this is not high, and anyone trying to get their feet wet in contributing to Freqtrade is more than welcome to give this a try (we will support in case of problems - a draft PR shall be used to collaborate / investigate issues).

@xmatthias xmatthias added the Enhancement Enhancements to the bot. Get lower priority than bugs by default. label Apr 28, 2020
@av1nxsh
Copy link
Contributor

av1nxsh commented Mar 3, 2021

hey, i am new to git
which command gives the status of the exchange and how do i test my code for maintenance test case

thank you.

@xmatthias
Copy link
Member

there is no command for this at this moment - and testing for this properly is rather difficult - unless the exchange is "really" down.

Also, it might be difficult to implement as not all exchanges will have this endpoint - but for freqtrade, we'll need to handle all cases.

@werkkrew
Copy link

Is this still considered a valuable "good first issue"?

@xmatthias
Copy link
Member

kindof - yeah ... although it's a more advanced first issue (good to start with freqtrade, but not good for programming novices), as i think we'd want to check the "exchange status" endpoint as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Enhancements to the bot. Get lower priority than bugs by default. Good first issue Good issues to get familiar and start contributing to freqtrade RPC Telegram, ReST API, external control, ...
Projects
None yet
Development

No branches or pull requests

6 participants
@werkkrew @xmatthias @sanket-k @hroff-1902 @av1nxsh and others