Problems with Depth Cache

[andrius-t]

I've been running Depth Cache for a while now. And the code broke two times, these are the errors:

 node-binance-api.js:280

                 for ( obj of depth.b ) { //bids
                                    ^
 TypeError: depth.b is not iterable

node-binance-api.js:603
                 for ( let depth of messageQueue[symbol] ) {
                                                ^

TypeError: messageQueue[symbol] is not iterable

I think Depth Cache has some issues at the moment. Or is it my fault? This is the code I'm using

binance.websockets.depthCache(socketArray, function(symbol, depth) {
    let bids = binance.sortBids(depth.bids);
    let asks = binance.sortAsks(depth.asks);
    let bid = binance.first(bids);
    let ask = binance.first(asks);
    prices[symbol] = prices[symbol] || {}
    prices[symbol].ask = parseFloat(bid),
    prices[symbol].bid = parseFloat(ask)
});

[jaggedsoft]

Thank you for the report! More error handling will be added

[andrius-t]

Also, don't know if this helps, but this usually happens when a socket is reconnecting. For example: WebSocket reconnecting: ethbtc@depth

[jaggedsoft]

Yes thank you, that absolutely helps. That tells me the problem is when it is unable to reconnect the first time. So now we should be able to tell it to keep trying to reconnect until it works

Did this happen last night when binance went down?

The problem is with /depth (request) not @depth (websocket)

[andrius-t]

It happened today few times. Not sure why but there are moments when socket connections go down. Probably due to the moments when there are high loads on Binance servers.

[jaggedsoft]

Thanks. I intend to fix this as soon as I can. I'm behind on some projects right now so it will take me a bit to get caught up

[bitcoinvsalts]

This issue might be related to my WebSocket connection closed! problems. @jaggedsoft you are doing a great job!

[josepgomes]

Hi,

Don't know if this is the perfect way to correct this problem, but can help a lot of people until official fix is released:

line 603

info[symbol].firstUpdateId = json.lastUpdateId;
depthCache[symbol] = depthData(json);
for ( let depth of messageQueue[symbol] ) {
    depthHandler(depth, json.lastUpdateId);
}
delete messageQueue[symbol];
if ( callback ) callback(symbol, depthCache[symbol]);

To

if (messageQueue != null && typeof messageQueue[symbol] === 'object') {
    info[symbol].firstUpdateId = json.lastUpdateId;
    depthCache[symbol] = depthData(json);
    for ( let depth of messageQueue[symbol] ) {
        depthHandler(depth, json.lastUpdateId);
    }
    delete messageQueue[symbol];
    if ( callback ) callback(symbol, depthCache[symbol]);
}

PS: It worked for me, no more errors :)

[jaggedsoft]

Thank you very much @josepgomes If that code is tested and working I can include it in the next release

[AlessandroSpallina]

when will be added this fix in the library?

[jaggedsoft]

I haven't had time to test it yet. I was hoping @josepgomes would respond if it is working properly & resolved

[josepgomes]

The 'if' that i added revolved completely the issue in my software. We are still developing using this library and the error did not happen again :)

Still, i hope someone test it too! Pretty fast to test it if you already have the request implemented.

[jaggedsoft]

Thank you very much! I will push an update right now

[jaggedsoft]

Pushing another update which fixes data corruption on reconnect

[jmarto]

@jaggedsoft just started using this wrapper - great work! However I am getting this issue still with version 0.4.11. Rolling back it seems to be introduced in 0.4.9, where i get the following:

/node_modules/node-binance-api/node-binance-api.js:837
for (let depth of messageQueue[symbol])
                              ^

TypeError: messageQueue[symbol] is not iterable

I've rolled back to 0.4.8 for now which seems to be working fine.

[jaggedsoft]

@jmarto Thank you very much! This is possible due to contributors like @keith1024 who help keep us going. Will look in to it.

[jmarto]

Thanks!

[bkrypt]

@jmarto @jaggedsoft I'll have a look at this right now. Will provide updates on my findings as soon as I have any.

@jmarto Could you please provide a snippet of what your call to websockets.depthCache looks like, along with what your first parameter (symbols) is? Thanks.

[jmarto]

Interestingly enough this error doesn't appear when using the package on npm-runkit...

[jmarto]

My code was exactly as per the demo. Reproduced here:

const BinanceAPI = require('node-binance-api'); //https://www.npmjs.com/package/node-binance-api
  BinanceAPI.websockets.depthCache(['XRPBTC','LTCBTC'], (symbol, depth) => {
    let bids = BinanceAPI.sortBids(depth.bids);
    let asks = BinanceAPI.sortAsks(depth.asks);
    console.log(symbol+" depth cache update");
    console.log("best bid: "+BinanceAPI.first(bids));
    console.log("best ask: "+BinanceAPI.first(asks));
  });

[bkrypt]

@jmarto Does a symbol pair perhaps appear more than once in your pairArray?

[jmarto]

It wasn't. I was initially passing in as per my edit with simply ['XRPBTC','LTCBTC']. But would you believe that it has just decided to work with v0.4.11? Can't figure out why, but I'm sure it was my fault. Sorry to bother! Thanks for following up so quickly.

[bkrypt]

Ah great, I'm glad to hear it's working now. The only way I could see to reproduce that error, and my tests confirmed, was to have a duplicate symbol somewhere in that array. So I'll add in logic to avoid that situation, just in case. No bother, it at least helped to discover the above case. :)

[jmarto]

@keith1024 Yes you're right. That does in fact reproduce it for me too.

I went through my undo stack and found the problem again. It turns out I was effectively doing as you said, but to simplify it, I was essentially making two calls to .depthCache() for the same symbol, like so:

const binance = require("node-binance-api");
function openBinanceSocket() {
  binance.websockets.depthCache(['BNBBTC'], (symbol, depth) => {
    let bids = binance.sortBids(depth.bids);
    let asks = binance.sortAsks(depth.asks);
    console.log(symbol+" depth cache update");
    console.log("best bid: "+binance.first(bids));
    console.log("best ask: "+binance.first(asks));
  });
};
openBinanceSocket();
openBinanceSocket();

That's my bad. Thanks for you help finding it.

[bkrypt]

@jmarto Excellent, thanks for the update. That makes sense that it would cause the same error. If you delayed that second call however, it would work, but then you'd have two sockets open providing duplicate data.

@jaggedsoft Perhaps as a more bulletproof fix, we can discuss having the API throw a more descriptive error when:

1. Attempting to subscribe to, technically, a duplicate data stream, or
2. A symbol appears more than once in any of the applicable websockets functions.

I say technically above as it would only detect exact duplicate subscriptions as an error. ie.

// Fine: This is the first call, so no possible duplicates
websockets.depthCache(['BNBBTC'], ...);

// Error: Subscription results in a duplicate data stream
websockets.depthCache(['BNBBTC'], ...);

// Fine: You are essentially receiving duplicate data for BNBBTC on this socket,
// but overall, the stream is not a duplicate
websockets.depthCache(['BNBBTC', 'XRPBTC'], ...);

The alternative to point 1, if we don't want to restrict users to only a single point of subscription, would be to just attach their callback to the already existing WebSocket instance.

What do you think?