Skip to content
This repository has been archived by the owner on Oct 30, 2023. It is now read-only.

Websocket inconsistency #71

Closed
michelesandroni opened this issue Feb 2, 2018 · 20 comments
Closed

Websocket inconsistency #71

michelesandroni opened this issue Feb 2, 2018 · 20 comments
Assignees

Comments

@michelesandroni
Copy link

Hello,

I've just noticed that, after subscribing to the candlestick websocket (for example) for symbol 'BTCUSDT' the library seems to subscribe to a websocket for each letter.
In order words, when listing all websockets 60 seconds (with a timeout) after subscribing them, this is what is shown:

ws subscription:
binance.websockets.candlesticks(['BTCUSDT'], '1m', (candlestickData) => { });

ws listing after 60 seconds:
let endpoints = binance.websockets.subscriptions();
for ( let endpoint in endpoints ) {
console.log('' + endpoint);
}

console output:
btcusdt@kline_1m
b@kline_1m
t@kline_1m
c@kline_1m
u@kline_1m
s@kline_1m
d@kline_1m

(tested on 0.4.8)

@jaggedsoft
Copy link
Member

Thanks for your report!
I can not reproduce this

binance.websockets.candlesticks(['BTCUSDT'], '1m', (candlestickData) => { });
setTimeout(()=>{
	let endpoints = binance.websockets.subscriptions();
	for ( let endpoint in endpoints ) {
		console.log(endpoint);
	}
}, 1000);

Result:
image

@jaggedsoft
Copy link
Member

jaggedsoft commented Feb 2, 2018

you have inspired me to add a verbose option so we can troubleshoot events happening inside the library / add extra debug logging

@jaggedsoft
Copy link
Member

jaggedsoft commented Feb 2, 2018

I believe what you want is this:

	for ( let endpoint of endpoints ) {
		console.log(endpoint);
	}

edit: might not work

@jaggedsoft jaggedsoft self-assigned this Feb 2, 2018
@michelesandroni
Copy link
Author

strange, cause i just now used your repro steps as they are and this was the output in my system:
btcusdt@kline_1m
b@kline_1m
t@kline_1m
c@kline_1m
u@kline_1m
s@kline_1m
d@kline_1m

@jaggedsoft
Copy link
Member

Try console.log(endpoints); instead of the loop

@michelesandroni
Copy link
Author

logging endpoints gives
{b@kline_1m: WebSocket, t@kline_1m: WebSocket, c@kline_1m: WebSocket, u@kline_1m: WebSocket, s@kline_1m: WebSocket, …}
b@kline_1m:WebSocket {domain: null, _events: {…}, _eventsCount: 5, _maxListeners: undefined, readyState: 1, …}
btcusdt@kline_1m:WebSocket {domain: null, _events: {…}, _eventsCount: 5, _maxListeners: undefined, readyState: 1, …}
c@kline_1m:WebSocket {domain: null, _events: {…}, _eventsCount: 5, _maxListeners: undefined, readyState: 1, …}
d@kline_1m:WebSocket {domain: null, _events: {…}, _eventsCount: 5, _maxListeners: undefined, readyState: 1, …}
s@kline_1m:WebSocket {domain: null, _events: {…}, _eventsCount: 5, _maxListeners: undefined, readyState: 1, …}
t@kline_1m:WebSocket {domain: null, _events: {…}, _eventsCount: 5, _maxListeners: undefined, readyState: 1, …}
u@kline_1m:WebSocket {domain: null, _events: {…}, _eventsCount: 5, _maxListeners: undefined, readyState: 1, …}

using 'of' instead of 'in' just throws an error

@jaggedsoft
Copy link
Member

I'm sorry but I'm really at a loss here
if we look at the candlesticks function:

candlesticks: function candlesticks(symbols, interval, callback) {
	for ( let symbol of symbols ) {
		let reconnect = function() {
			if ( options.reconnect ) candlesticks(symbol, interval, callback);
		};
		subscribe(symbol.toLowerCase()+'@kline_'+interval, callback, reconnect);
	}
},

Essentially all it's doing is this:

let symbols = ['BTCUSDT'];
for ( let symbol of symbols ) {
    console.log(symbol);
}

@jaggedsoft
Copy link
Member

I'm adding verbose to the next release, shows when new subscriptions are made, etc
You could try testing with reconnect: false in the options to eliminate if it's a reconnection issue

@jaggedsoft
Copy link
Member

Maybe you can try the chart function? I prefer it over candlesticks

@michelesandroni
Copy link
Author

chart shows the same behavior and also seem to use the same endpoint @kline_1m
the reconnect flag doesnt seem to affect the behavior

@michelesandroni
Copy link
Author

sorry, some more info about this: after closing and restarting nw.js, i cannot reproduce this one either. those zombie websockets seem to have been created at some point last night after the websocket was reconnected, but i can't tell exactly how or when. Let's keep this closed and I'll post more info once I have new details. Thanks!

@michelesandroni
Copy link
Author

The verbose flag sounds like a really good idea!

@jaggedsoft
Copy link
Member

Never heard of nw.js, it allows you to use this from the browser or what?

@michelesandroni
Copy link
Author

It used to be called node-webkit, it's basically chromium + node and enables users to turn html5 into cross-platform executables among other things. Similar to electronjs I guess (I want to try that one soon).
https://nwjs.io/

@jaggedsoft
Copy link
Member

jaggedsoft commented Feb 2, 2018

sweet thanks, keep me updated if you have any problems etc
I'm probably gonna try electron myself, looks awesome!

@michelesandroni
Copy link
Author

thank you!

@michelesandroni
Copy link
Author

Now I can definitely reproduce the issue.
The behavior is not always consistent (sometimes it generates the error immediately, sometimes not).
/*
Output:
index.html:105 Endpoint: btcusdt@kline_1m
index.html:105 Endpoint: b@kline_1m
index.html:105 Endpoint: t@kline_1m
index.html:105 Endpoint: c@kline_1m
index.html:105 Endpoint: u@kline_1m
index.html:105 Endpoint: s@kline_1m
index.html:105 Endpoint: d@kline_1m
index.html:105 Endpoint: btcusdt@kline_1m
index.html:105 Endpoint: b@kline_1m
index.html:105 Endpoint: t@kline_1m
index.html:105 Endpoint: c@kline_1m
index.html:105 Endpoint: u@kline_1m
index.html:105 Endpoint: s@kline_1m
index.html:105 Endpoint: d@kline_1m
index.html:105 Endpoint: btcusdt@kline_1m
index.html:105 Endpoint: b@kline_1m
index.html:102 Uncaught TypeError: Cannot read property 'code' of undefined
at WebSocket. (C:\Test\node_modules\node-binance-api\node-binance-api.js:231:62)
at WebSocket.emit (events.js:159:13)
at WebSocket.finalize (C:\Test\node_modules\ws\lib\WebSocket.js:182:41)
at WebSocket.terminate (C:\Test\node_modules\ws\lib\WebSocket.js:389:12)
at Object.terminate (C:\Test\node_modules\node-binance-api\node-binance-api.js:770:8)
*/

class Tester {
start() {
this.binance = require('node-binance-api');
this.testWs();
}
testWs() {
var thiz = this;
this.binance.candlesticks('BTCUSDT', '1m', (error, ticks, symbol) => {
// ...
thiz.binance.websockets.candlesticks(['BTCUSDT'], '1m', (candlestickData) => {
// ...
});
}, {
limit: (267 + 14 + 1),
endTime: Date.now()
});
setTimeout(function() {
let endpoints = thiz.binance.websockets.subscriptions();
for ( let endpoint in endpoints ) {
console.log('Endpoint: ' + endpoint);
thiz.binance.websockets.terminate(endpoint);
}
setTimeout(function() {
thiz.testWs(); // test recursively ...
}, 5000);
}, 5000);
}
}

let myTester = new Tester();
myTester.start();

@jaggedsoft
Copy link
Member

This project is only designed for node js. I can not afford to research what I would need to do to make it work with nwjs or electron

@michelesandroni
Copy link
Author

Hi Jaggedsoft,

please try saving the code from my previous comment as test.js, place it in some folder where you can install 'node-binance-api' via npm, then run "node test.js".

I just did it and i can repro the issue using node v8.9.4 (same results as nw.js).

@jaggedsoft
Copy link
Member

I cannot reproduce this
Possibly related to jaggedsoft#81

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants