thibaultyou / tradingview-alerts-processor

Minimalist service designed to execute TradingView webhooks and process them to cryptocurrencies exchanges.
MIT License
79 stars 23 forks source link

:recycle: Address low priority logging issue #26

Closed leifjones closed 3 years ago

leifjones commented 3 years ago

This seeks to address an issue with error logging. There are also some increases tests and some proposed tests as a conversation piece.

I was seeing messages like the following when there is a sell signal for a token that isn't yet held in a BinanceUS account:

Unable to fetch LTC ticker balance (may be 0) -> TypeError: Cannot read property 'free' of undefined.
Failed to close LTC/USD position.
Failed to open position on LTC/USD.

As one of the first users of this with BinanceUS - in case this would be a question, I can confirm that I was able to do manually triggered buy and sell signals:

[{"message":"exchanges|BinanceUS/[STUB] - WAVES ticker balance successfully fetched. -> {\"coin\":\"WAVES\",\"free\":\"0.36000000\",\"total\":0.36}","level":"debug","timestamp":"2021-09-12T03:13:47.814Z"},
{"message":"trading|Calculated 0.35961462389214177 WAVES/USD equivalent for 11.00 $US.","level":"debug","timestamp":"2021-09-12T03:13:47.815Z"},
{"message":"trading|Calculated 0.35961462389214177 WAVES/USD equivalent for 11.00 $US.","level":"debug","timestamp":"2021-09-12T03:13:47.815Z"},
{"message":"trading|BinanceUS/[STUB] - $$$ Closing 0.35961462389214177 WAVES/USD of open position / available balance (~ 11.00 $US).","level":"info","timestamp":"2021-09-12T03:13:48.754Z"},
{"message":"trading|Trade [omitted-uuid] executed in 2564 ms.","level":"info","timestamp":"2021-09-12T03:13:48.755Z"}]

===============

Notably, there are a number of todoed tests:

leifjones commented 3 years ago

I have got to acknowledge that, so far, this doesn't actually address handling of the error that arose. Rather, it breaks up code so that a "failure to open an order" error doesn't get logged when it was more of a "failure to close a position."

Actually identifying the precise location and cause of the error (cannot find a property of an undefined variable) may require some debugging. I have done some initial exploration of how to enable sandbox mode for an exchange on a separate branch but haven't yet made substantial progress on it.

I've updated the PR to include null checks. Now the output when trying to sell a token that isn't held is:

error:exchanges|BinanceUS/[STUB] - IOTA ticker balance missing (may be 0).
error:trading|BinanceUS/[STUB] - Failed to close IOTA/USD position.
error:trading|BinanceUS/[STUB] - Failed to create sell order on IOTA/USD.

If these changes aren't deemed worthwhile, it's fine. Honestly, I'm taking it as a way to familiarize myself with the code and get more confident with using TAP in action.

thibaultyou commented 3 years ago

Thanks for the update.

Let me test this once I've some free time and I'll merge this into master if you're done with this PR.

🙏

leifjones commented 3 years ago

@thibaultyou yep, I'm done with the changes I was planning

thibaultyou commented 3 years ago

Just tested it, it's good for me. I'll merge it now to master and update the Docker image once I'm logged on a non-ARM architecture.

Thank you for your contribution 🙏 .

thibaultyou commented 3 years ago

The Docker image has been updated.