Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

order status check via API throws error #19

Open
adityamertia opened this issue Feb 20, 2018 · 10 comments
Open

order status check via API throws error #19

adityamertia opened this issue Feb 20, 2018 · 10 comments

Comments

@adityamertia
Copy link
Contributor

adityamertia commented Feb 20, 2018

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
Copy link

tuloski commented Feb 27, 2018

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
Copy link
Contributor Author

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

@tuloski
Copy link

tuloski commented Feb 27, 2018

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
Copy link
Owner

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
Copy link
Contributor Author

adityamertia commented Mar 5, 2018

@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
Copy link

tuloski commented Mar 5, 2018

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

@adityamertia
Copy link
Contributor Author

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
Copy link

tuloski commented Mar 5, 2018

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
Copy link
Contributor Author

adityamertia commented Mar 8, 2018

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
Copy link

tuloski commented Mar 8, 2018

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

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

No branches or pull requests

3 participants