krystianity / node-bitstamp

bitstamp REST and WS API Node.js client :dollar:
MIT License
59 stars 41 forks source link

order status check via API throws error #19

Open adityamertia opened 6 years ago

adityamertia commented 6 years ago

Hi, Whenever i check for an orders status using await bitstamp.orderStatus(orderId).then(({status, headers, body}) => body); I keep getting following error even if i run it at 5 sec intervals:

(node:879) UnhandledPromiseRejectionWarning: Error: Must not exceed 60 calls per 60000 ms.
13|tradeBi |     at Promise (/home/ec2-user/crypto_arbitrage_latest/bitstamp/node_modules/node-bitstamp/lib/Bitstamp.js:85:31)
13|tradeBi |     at Promise._execute (/home/ec2-user/crypto_arbitrage_latest/bitstamp/node_modules/bluebird/js/release/debuggability.js:303:9)
13|tradeBi |     at Promise._resolveFromExecutor (/home/ec2-user/crypto_arbitrage_latest/bitstamp/node_modules/bluebird/js/release/promise.js:483:18)
13|tradeBi |     at new Promise (/home/ec2-user/crypto_arbitrage_latest/bitstamp/node_modules/bluebird/js/release/promise.js:79:10)
13|tradeBi |     at Bitstamp.call (/home/ec2-user/crypto_arbitrage_latest/bitstamp/node_modules/node-bitstamp/lib/Bitstamp.js:79:16)
13|tradeBi |     at Bitstamp.orderStatus (/home/ec2-user/crypto_arbitrage_latest/bitstamp/node_modules/node-bitstamp/lib/Bitstamp.js:218:21)
13|tradeBi |     at checkOrderStatus (/home/ec2-user/crypto_arbitrage_latest/bitstamp/tradeBitstamp.js:195:36)
13|tradeBi |     at Timeout._onTimeout (/home/ec2-user/crypto_arbitrage_latest/bitstamp/tradeBitstamp.js:90:31)
13|tradeBi |     at ontimeout (timers.js:458:11)
13|tradeBi |     at tryOnTimeout (timers.js:296:5)
13|tradeBi |     at Timer.listOnTimeout (timers.js:259:5)
13|tradeBi | (node:879) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 41)

Its run 15 times in 60000ms still i get this error. what could be wrong here?

tuloski commented 6 years ago

Are you sending other requests meanwhile? Because the cap is for all the request, not for a single call. And are you sure you are calling every 5s?

adityamertia commented 6 years ago

Yes there is no other instance running and the checkStatus is called every 5 secs. I still see this issue on and off.

tuloski commented 6 years ago

Try to debug, with "console.log("Calls in last minute: " + this.callsInLastMinute)" in line 83 of Bitstamp.js. You should see that increasing by one every 5 seconds, then reset every minute.

krystianity commented 6 years ago

The logic is straight forward, as tuloski has explained. I suspect you are calling more then 15 times. However besides debugging you can disable this behaviour by setting "rateLimit" to false in the options you pass the constructor - but this will return in API errors if you make to many calls.

adityamertia commented 6 years ago

@tuloski @krystianity I have just added this console.log and it helped in reproducing the issue. From the logs its clear that the timer held by node-bitstamp doesn't reset after a minute in one of the cases and hence this issue pops up. Check the log snippet below to verify the counter incrementing even after 1 minute has passed and hence starts throwing error. This however does happen in 1 in 20 iterations. logsnippet.txt

tuloski commented 6 years ago

I tried and it resets for me. Which version of nodejs are u using? Is running in a browser?

adityamertia commented 6 years ago

It does reset for me 19 out of 20 times .... but once in a while it does not ...as seen in the log attached and then it starts giving error. I am using node v9.4.0 running on linux.

tuloski commented 6 years ago

I'm not a big fan of setInterval, because they are known to create problems. Try to debug with a console.log also inside the setInterval function. The logic is so easy that it has to be some sneaky thing.

adityamertia commented 6 years ago

You refering to setInterval inside library? In Bitstamp.js:

// 600 requests max per 10 minutes
const MAX_CALL_WINDOW = 60 * 1000; // 10 minutes
const MAX_CALLS_PER_WINDOW = 60;

this._intv = setInterval(() => {
             this.callsInLastMinute = 0;
         }, MAX_CALL_WINDOW);
tuloski commented 6 years ago

Yeah...that should reset the calls per minute every minute.