BitBotFactory / MikaLendingBot

Automated lending on Cryptocurrency exchanges Poloniex and Bitfinex
http://poloniexlendingbot.readthedocs.io/en/latest/index.html
MIT License
1.11k stars 344 forks source link

Error text too long ... #402

Closed mike4001 closed 7 years ago

mike4001 commented 7 years ago

Hi

Today I had the error (see screenshot). I think you should add a maximum amount of characters since Poloniex return a complete website for this one.

unbenannt

tamaskan commented 7 years ago

i think its better to add a exception like https://github.com/BitBotFactory/poloniexlendingbot/commit/38ac656d5ce37c8624f4686096589680b911a3bf

rnevet commented 7 years ago

Yeah, saw that one as well... would add it to the exceptions like tamaskan suggested. A PR would be appreciated.

tisdall commented 7 years ago

Maybe we should extend #392 to include error code 522? Apparently that's a Cloudflare specific error code that seems pretty similar to 502: https://support.cloudflare.com/hc/en-us/articles/200171906-Error-522-Connection-timed-out

It looks like there's error codes 520 through to 526 that Cloudflare may return resulting in a whole web page. We should just ignore the body with any of those errors...

... just clicked @tamaskan 's link and see they're saying the same thing about extending #392 . ;)

mchl18 commented 7 years ago

wouldn't this solve it? how to reproduce the error?

lendingbot.py:133

if ex.code == 502 or ex.code in range(520, 526, 1):  # 502 and 520-526 Bad Gateway so response is likely HTML from Cloudflare
                    polo_error_msg = ''
mchl18 commented 7 years ago

makin a PR, cant test it though. when does this happen?

utdrmac commented 7 years ago

@Evanito Please re-open. @mchl18 Please re-examine. I received this twice today.

screen shot 2017-08-14 at 3 39 16 pm
mchl18 commented 7 years ago

So that is the cloudflare html that is supposed to be blocked? @utdrmac any console errors?

tisdall commented 7 years ago

Yes, Cloudflare... The code committed should have fixed that, though.

mchl18 commented 7 years ago

I don't think I'll be able to provoke this, but should be fixed with PR... image it should be range(520, 527, 1) though but yours should be caught

mchl18 commented 7 years ago

if you can provoke it, can you debug into it somehow? @utdrmac

utdrmac commented 7 years ago

Ah. My screenshot is from the recent BITFINEX merge. I see your PR only deals with Poloniex. Can you add the cloudflare checks to Bitfinex too?

mchl18 commented 7 years ago

are you lending with bitfinex?

utdrmac commented 7 years ago

Yep. Polo is dead on lending. Currently 0 BTC demand.

utdrmac commented 7 years ago

Lending ETH, BCH and BTC on bitfinex with the recent merge.

mchl18 commented 7 years ago

Okay, i really can't test this bc I am not lending on bitfinex right now... but

Bitfinex.py:46

def _request(self, request, command, payload=None, verify=True):
        try:
            r = {}
            if (request == 'get'):
                r = requests.get(self.url + command, timeout=self.timeout)
            else:
                r = requests.post(self.url + command, headers=payload, verify=verify, timeout=self.timeout)

            if r.status_code != 200:
                if (r.status_code in [502, 504, 522]):
                    raise ApiError('API Error ' + str(r.status_code) +
                                   ': The web server reported a bad gateway or gateway timeout error.')
                else:
                    raise ApiError('API Error ' + str(r.status_code) + ': ' + r.text)

            return r.json()

        except Exception as ex:
            ex.message = ex.message if ex.message else str(ex)
            ex.message = "{0} Requesting {1}".format(ex.message, self.url + command)
            raise ex

could also be

Bitfinex.py:55 if (r.status_code == 502 or in r.status_code in range(502, 527, 1)):

since they seem to behave the same

also Poloniex.py should be updated to reflect that

utdrmac commented 7 years ago

I'll test it

[root@gasgiant bitfinex]# git diff modules/Bitfinex.py
diff --git a/modules/Bitfinex.py b/modules/Bitfinex.py
index 4a281da..f53c584 100644
--- a/modules/Bitfinex.py
+++ b/modules/Bitfinex.py
@@ -52,7 +52,7 @@ class Bitfinex(ExchangeApi):
                 r = requests.post(self.url + command, headers=payload, verify=verify, timeout=self.timeout)

             if r.status_code != 200:
-                if (r.status_code in [502, 504, 522]):
+                if (r.status_code == 502 or r.status_code in range(502, 527, 1)):
                     raise ApiError('API Error ' + str(r.status_code) +
                                    ': The web server reported a bad gateway or gateway timeout error.')
                 else:
mchl18 commented 7 years ago

something is wrong with my branch... getting an error that I don't know...

/poloniexlendingbot/modules/Bitfinex.py", line 6, in <module>
    import requests

ImportError: No module named requests
rnevet commented 7 years ago

@mchl18 You need to install requests. It's explained in other issues.

mchl18 commented 7 years ago

maybe you reopen this issue for the sake of bitfinex testing. I have just pushed to my fork, the code should fix the issue, although I must say I am not really able to test, since I wanted to suspend lending on polo and do not have anything on bitfinex right now. My commit history has some commits in there that were pointless, I was getting used to Github workflow, sorry. Also I have noticed that error handling is being used differently for each Polo and Bitfinex, maybe a shared error message would be reasonable.

mchl18 commented 7 years ago

codebeat declined the code... I don't know the constraints of the new codebeat testing

tisdall commented 7 years ago

@mchl18 - I'm glad you noticed that it was missing the 526 error code (not sure how much that happens, but also a Cloudflare error code).

rnevet commented 7 years ago

"maybe a shared error message would be reasonable." I agree.

Did you make a PR? I don't see one.

mchl18 commented 7 years ago

Well, here is the PR with the basic changes. The last commit is relevant, as I said, I had some issues with workflow (still new here)

rnevet commented 7 years ago

That's not an issue, we'll squash the commits and merge.