Connection to multiple market - PANIC

dklodner commented 5 years ago

Hi, I am trying to connect to multiple markets but the program keeps crashing in UpdateWidth function (orderbook.go).

Here's the code to reproduce this issue. It takes a minute or so to get the error message. https://paste.ubuntu.com/p/yrMhhDBRsn/

What am I doing wrong? Thanks for your help...

David

JacobPlaster commented 5 years ago

Hi @dklodner,

Thanks for reporting this, your code is perfectly fince. The code that calculates the checksum of the orderbook had a small bug which only caused a problem when the orderbooks got thin. I have just merged a fix for this which you can see here https://github.com/bitfinexcom/bitfinex-api-go/pull/145.

Please could you make sure you have the latest commit on master an try again. Let me know if you have any more problems with it.

dklodner commented 5 years ago

Hii @JacobPlaster, it's still an issue ...

` 2019/01/14 14:44:53 Orderbook 'tEOSUSD' checksum verification successful. 2019/01/14 14:44:53 [DEBUG]: {"event":"unsubscribed","status":"OK","chanId":74264} 2019/01/14 14:44:53 [DEBUG]: [74264,"cs",-2129011831] 2019/01/14 14:44:53 MSG RECV: &websocket.UnsubscribeEvent{Status:"OK", ChanID:74264} panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x626989]

goroutine 103 [running]: github.com/bitfinexcom/bitfinex-api-go/v2/websocket.(*Client).handleChecksumChannel(0xc420158180, 0x12218, 0xffffffff8119db89, 0x0, 0x0)

`

dklodner commented 5 years ago

@JacobPlaster, another one with concurrent map read and write error: https://pastebin.ubuntu.com/p/yWV7fTHkP6/

dklodner commented 5 years ago

Sorry to keep spamming, but it's throwing different error messages:

2019/01/14 17:45:22 [DEBUG]: [147316,[32.78,0,1]] panic: runtime error: index out of range

goroutine 169 [running]: github.com/bitfinexcom/bitfinex-api-go/v2/websocket.(*Orderbook).Checksum(0xc4208e9280, 0xc4201b8090) .../Golang/src/github.com/bitfinexcom/bitfinex-api-go/v2/websocket/orderbook.go:75 +0x5c6

JacobPlaster commented 5 years ago

Ok thanks @dklodner thats no problem, thanks for pasting them here because I was struggling to reproduce this. Im going to look into this again for you

dklodner commented 5 years ago

Another one @JacobPlaster

https://pastebin.ubuntu.com/p/3tXbbcjhNR/

JacobPlaster commented 5 years ago

@dklodner I've just merged a PR which hopefully fixes all of the issues you have pointed out above (see PR https://github.com/bitfinexcom/bitfinex-api-go/pull/146). This adds some extra error catching and sync locking.

I've been running your script for 3 hours on my changes and havent seen any crash's so please let me know if you still get any errors. Also, if you do see errors in the meantime try disabling the crc32 orderbook verification for a short term fix i.e p.ManageOrderbook = false

dklodner commented 5 years ago

Thank you @JacobPlaster for your work on this issue. There's a new error with concurrent map write. The program seems to try to connect multiple times to the same ticker...

https://pastebin.ubuntu.com/p/HVjgck8mxt/

JacobPlaster commented 5 years ago

You're welcome. I'll take a look into this one tomorrow, thanks for the pastebin.

JacobPlaster commented 5 years ago

OK, it seems like the are a few areas that need a sync map lock. Ill get that merged in soon. @dklodner Is everything working correctly for you with manageorder book disabled?

dklodner commented 5 years ago

It seems stable with manageorderbook disabled.

dklodner commented 5 years ago

Hi @JacobPlaster. Would you please take a look at this issue? Is there any way how to handle 429 status code? Also program panics after all reconnection attempts finish.

https://paste.ubuntu.com/p/49w26kFYmq/

JacobPlaster commented 5 years ago

Hi @dklodner sorry for the late reply. Ill check that out for you

JacobPlaster commented 5 years ago

@dklodner again sorry for the late reply. Did you manage to sort this out? It seems like the above error that you posted was caused by our API gateway rate limits

dklodner commented 5 years ago

API rate limiting can't crash the program, don't you think? It looks like the library isn't handling 429 rate limiting error messages (increasing reconnection timeouts?) and it panics once the reconnection process ends after predefined number of tries.

JacobPlaster commented 5 years ago

@dklodner yes I agree, but It doesnt seem like its the rate limiting bad hanshake is causing the crash here. The library attempts the reconnect which is to be expected but does not panic until it exhausts all of its attempts and then finally proceeds to kill the process. I think this is more of a problem with the lib not shutting down gracefully. I wonder if you would still get this problem if you increased the reconnection timeouts/attempts so that the lib re-establishes the connection using something like this:

p := websocket.NewDefaultParameters()
// Enable orderbook checksum verification
p.ReconnectAttempts = 100
p.ReconnectInterval = time.Second * 60
c := websocket.NewWithParams(p)

err := c.Connect()
if err != nil {
    log.Fatal("Error connecting to web socket : ", err)
}

The bitfinex API limits can vary in a range of 10 to 90 requests per minute so setting the interval to be a minute should reset the limits. In the meantime though ill definitely look into the shutdown logic of the lib because even though the library is shutting down it should not be causing a panic

dklodner commented 5 years ago

That's what I ended up doing. It would be great if you were able to find a permanent solution. Thanks for your help.

bitfinexcom / bitfinex-api-go

Connection to multiple market - PANIC #144