krystianity / node-bitstamp

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

Error not thrown properly by node-bitstamp #18

Closed adityamertia closed 5 years ago

adityamertia commented 6 years ago

Hi, This seems to be a regression from the latest changes done 2 days back. Kindly confirm. I am getting following errors if there is insufficient balance.

13|tradeBi | (node:12851) UnhandledPromiseRejectionWarning: Error: [object Object]
13|tradeBi |     at Request.request [as _callback] (/home/ec2-user/crypto_arbitrage_latest/bitstamp/node_modules/node-bitstamp/lib/Bitstamp.js:115:35)
13|tradeBi |     at Request.self.callback (/home/ec2-user/crypto_arbitrage_latest/bitstamp/node_modules/request/request.js:186:22)
13|tradeBi |     at Request.emit (events.js:160:13)
13|tradeBi |     at Request.<anonymous> (/home/ec2-user/crypto_arbitrage_latest/bitstamp/node_modules/request/request.js:1163:10)
13|tradeBi |     at Request.emit (events.js:160:13)
13|tradeBi |     at IncomingMessage.<anonymous> (/home/ec2-user/crypto_arbitrage_latest/bitstamp/node_modules/request/request.js:1085:12)
13|tradeBi |     at Object.onceWrapper (events.js:255:19)
13|tradeBi |     at IncomingMessage.emit (events.js:165:20)
13|tradeBi |     at endReadableNT (_stream_readable.js:1101:12)
13|tradeBi |     at process._tickCallback (internal/process/next_tick.js:152:19)
13|tradeBi | (node:12851) 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: 4)

@krystianity @nicolasgarnier kindly confirm.

krystianity commented 6 years ago

Yup the typo slipped, I have merged your PR and released 2.0.1

adityamertia commented 6 years ago

@krystianity I have upgraded to v2.0.1 but it doesn't fix the above error. Can you check pls? Can be easily reproduced by generating "insufficient balance".

adityamertia commented 6 years ago

@krystianity can you pls update on this issue?

nicolasgarnier commented 6 years ago

@adityamertua looks like you are simply (maybe) not catching the rejected promise. Can you show your code that calls this? You should have a .catch(error => { /* handle error here */ }) in there.

adityamertia commented 6 years ago

@nicolasgarnier Btw this error started only after the changes done that created typo error. Prior to that i was getting insufficient balance exception. However below is the code and the output if i catch the error:

async function bitstampLimitBuy(size, price, symbol) {
  var price = parseFloat(price.toFixed(8));
  logger.info(`sending limit buy order with price: ${price} and size: ${size}`)
  var jsonBody = await bitstamp.buyLimitOrder(size, price, symbol).then(({
    body: data
  }) => data)
  .catch((err)=>{
    logger.info(`ERROR: ${JSON.stringify(err)}`)
  });
  return jsonBody;
};
[2018-02-23T11:07:59.836Z] - info: tradeBitstamp.js: sending limit buy order with price: 0.01947802 and size: 10
[2018-02-23T11:08:00.331Z] - info: tradeBitstamp.js: ERROR: {}

If you notice that the issue that i reported above returns the error as an object i.e. UnhandledPromiseRejectionWarning: Error: [object Object] And this was returning a proper error in previous versions, instead of [object Object] it was throwing {"__all__":["You need 0.19526720 BTC to open that order. You have only 0.00394619 BTC available. Check your account balance for details."]}. I suspect some issue with the library as even after catching the error i get nothing in the error object. Also if i comment the line Bitstamp.js:115 which is as below:

// There was an API request error.
                 // e.g. Insufficient funds in case of withdrawal.
                 /*if (body.status === "error") {
                     return reject(new Error(body.reason));
                 }*/

Then i get the above catched error as below: ERROR: {"__all__":["You need 0.19526720 BTC to open that order. You have only 0.00394619 BTC available. Check your account balance for details."]} Which means that return reject(new Error(body.reason)); is the culprit. I tried using return reject(body.reason); which works but new Error(body.reason) doesn't, which looks wierd to me.

adityamertia commented 6 years ago

@krystianity @nicolasgarnier any update?

nicolasgarnier commented 6 years ago

I think you are facing this: https://stackoverflow.com/questions/18391212/is-it-not-possible-to-stringify-an-error-using-json-stringify

Using the original code, can you try changing your code with following:

  .catch((err)=>{
    logger.info(`ERROR: ${JSON.stringify(err.message)}`);
  }

and:

  .catch((err)=>{
    logger.info('ERROR:', err.message);
  }

and (usually how I do it):

  .catch((err)=>{
    logger.info('ERROR:', err);
  }
krystianity commented 6 years ago

@adityamertia those rejects are new to the client, as I didnt want them in the first place, but the community wanted them; they have been merged in the last releases.

The way you are handling the errors is strange, when going for await you should try/catch and not promise catch. I will reply to the empty error on the other issue.

adityamertia commented 6 years ago

@nicolasgarnier I tried all logger.info modifications mentioned by you but the issue persists. I still feel it has to do with the latest changes. @krystianity I did however tried what you have mentioned as well. Following is the code using node-bitstamp@v2.1.0.

async function bitstampLimitBuy(size, price, symbol) {
    logger.info(`sending limit buy order with price: ${price} and size: ${size}`)
    try{
      var jsonBody = await bitstamp.buyLimitOrder(size, price, symbol).then(({
          body: data
      }) => data)
    } catch (err) {
        logger.info(`ERROR: ${JSON.stringify(err.message)}`)
        logger.info('ERROR:', err);
        logger.info('ERROR:', err.message);
    }
    return jsonBody;
  };

output:

[2018-03-03T07:20:25.080Z] - info: tradeBitstamp.js: sending limit buy order with price: 0.01947802 and size: 10
[2018-03-03T07:20:25.443Z] - info: tradeBitstamp.js: ERROR: "[object Object]"
[2018-03-03T07:20:25.444Z] - info: tradeBitstamp.js: ERROR:
[2018-03-03T07:20:25.444Z] - info: tradeBitstamp.js: ERROR:

The error above is to be thrown because size=10 which (in my case) should create insufficient balance.

adityamertia commented 6 years ago

@krystianity @nicolasgarnier Any leads on this issue?

adityamertia commented 6 years ago

@krystianity @nicolasgarnier following up on this. Did u get a chance to look into this?

sonic1k commented 6 years ago

Hi, same problem here, trying to set an order less than 5 dollars I get: (node:15072) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): TypeError: Cannot read property '0'...... for debugging I tried to change the line in bitstamp.js like adityamertia did, and: return reject(new Error(body.reason)); this line produce the error return reject(body.reason); this other line does not produce the error I'm trying to handle the error with this code: try { orderResponse = await this.bitstamp.buyLimitOrder(amount, price, currency_pair) .then( function ({body:data}) { return data; }) return orderResponse; } catch (error) { console.log(error.__all__[0]); return error; } but nothing change, the rejection is still unhandled.

Anybody found a workaround or a solution?

adityamertia commented 6 years ago

@sonic1k I think it has something to do with the changes done a month back. I am also waiting for a resolution on the same. @krystianity @nicolasgarnier can you spare sometime to recheck the same?

brianconnoly commented 6 years ago

Could not get error message

Error: [object Object] at Request.request [as _callback] (/Users/vladislavkorotun/repos/zogras/zeco/node_modules/node-bitstamp/lib/Bitstamp.js:120:35) at Request.self.callback (/Users/vladislavkorotun/repos/zogras/zeco/node_modules/request/request.js:186:22) at Request.emit (events.js:160:13) at Request.<anonymous> (/Users/vladislavkorotun/repos/zogras/zeco/node_modules/request/request.js:1163:10) at Request.emit (events.js:160:13) at IncomingMessage.<anonymous> (/Users/vladislavkorotun/repos/zogras/zeco/node_modules/request/request.js:1085:12) at Object.onceWrapper (events.js:255:19) at IncomingMessage.emit (events.js:165:20) at endReadableNT (_stream_readable.js:1101:12) at process._tickCallback (internal/process/next_tick.js:152:19)

krystianity commented 5 years ago

Should be fixed in 2.2.0