Skip to content

Conversation

@Kurairaito
Copy link
Contributor

This is an attemp to fix #61

In my case, the bot run on a Synology NAS. When I used download manager (on NAS), (wich take 100% of my bandwith), my bot was sometimes stuck too.

With this fix, I didn't experienced the same issue, but I'm not sure if it correct it completely

This is an attemp to fix BitBotFactory#61

In my case, the bot run on a Synology NAS. When I used download manager (on NAS), (wich take 100% of my bandwith), my bot was sometimes stuck too.

With this fix, I didn't experienced the same usue, but I'm not sure if it correct it completely
@Evanito Evanito added the Queued label May 15, 2016
@Evanito
Copy link
Member

Evanito commented May 15, 2016

LGTM 👍

@rnevet
Copy link
Collaborator

rnevet commented May 15, 2016

I don't think you should have exception handling inside there.

@Evanito
Copy link
Member

Evanito commented May 15, 2016

With the exception handling, won't it log an error saying what went wrong and then continue as normal, instead of causing failure?
By having the exception set jsonRet = "" then we don't have an error where jsonRet is undefined or just mimics the last message.

@rnevet
Copy link
Collaborator

rnevet commented May 15, 2016

There is already error handling in the main bot loop, which also includes a sleep so the bot doesn't hammer the server.
I think that returning "" will anyway crash the bot just in another place because this isn't a valid response for the request - i.e. the code does more or less nothing except might generate an extra error log and a misleading error trace.

@Evanito
Copy link
Member

Evanito commented May 15, 2016

Hmm, you are probably right.

Close merge if you are sure.

@Kurairaito
Copy link
Contributor Author

Well on worst case, the bot crash and an other script can relaunch it.
Actually it is stuck at that line and you must monitor output file to see
if it is working or not, then kill it, and relaunch it. Maybe we could just
terminate the process if entering in exception ?

On Sun, May 15, 2016, 22:40 Evan [email protected] wrote:

Hmm, you are probably right.

Close merge if you are sure.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#67 (comment)

@rnevet
Copy link
Collaborator

rnevet commented May 16, 2016

I noticed that you only added the timeout to the private API calls, I created another pull request with a more general solution that adds the timeout on the socket level so all requests are covered.
Here:
#69

@rnevet rnevet closed this May 16, 2016
@rnevet rnevet removed the Queued label May 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bot is not working sometimes

3 participants