failed market buy errors out instead of throwing actual response

[programmrz]

After attempting market buy that exceeds available asset:

/home/xxxxx/public_html/demo/node_modules/node-binance-api/node-binance-api.js:89 if ( response && response.statusCode !== 200 ) throw response; ^ [object Object]

[jaggedsoft]

Thank you for the report! I will look in to this

[gnolmon]

I have same problem like that in method market, limit buy/sell order all the same

[kevflynn]

Same here - not be able to catch these errors is equally problematic. We should refactor how we throw errors with the callback. Makes it super hard to debug and super inefficient to crash the entire code base because an order fails.

[charlesdarkwind]

Any values that are well in range for a valid order are also throwing in my case. edit : thats because total value of order was under .001 BTC.

replacing "throw response" by "console.error(response.body)" is good enough for me atm

[jaggedsoft]

Perhaps someone can review this commit: https://github.com/jaggedsoft/node-binance-api/commit/228e2af10843bb871f86e67399003706fcdb6c75 We can roll it back if that was a breaking change Or even better, hopefully someone can propose a better solution

[jaggedsoft]

Can you guys also specify what kind of errors its throwing so I can plan to handle those cases

[jaggedsoft]

@hems Any chance you could offer some assistance? Unfortunately I am not available for the next few days

[hems]

@jaggedsoft absolutely!

@charlesdarkwind i believe a "console.log" isn't a good solution as this wouldn't be "catchable" in a production environment and would pollute the logs.

I believe the right to do is:

1 - Wrap your call to the API on a "try / catch" and catch the "error" object, if the error object constains "statusCode" it's because it didn't return "200 OK", so you should look into "error.statusCode" and "e.message" in order to see what is happening.

For instance if you wanted a console log you would do

try {
   my_api_call(....)
} catch ( e ) {
  if( e.statusCode ) {
    console.error( "something went wrong, statusCode isnt 200" )
    console.error( "e.statusCode:", e.statusCode )
    console.error( "e.message:", e.message )
  }
}

If you using a "Promises" library like "bluebird" you would then catch the error with the ".error()" method.

I think this patch had two major problems:

1 - We should have called callback( error ) instead of throwing the error, my bad! 2 - We should have bumped the version not in a minor version

While we don't fix that i believe you guys could revert to the previous version, that would solve the issue.

[hems]

I was just testing out if you guys are really using the library :P

Jokes aside, i overlooked two things: 1 - We should use the standard callback( error, data ) and not throw errors 2 - We should have bumped versions

So I created this new pull request which hopefully solves the situation: pull request #53

Please let me know if that looks better to you!

As i said: I believe we should follow node.js standard of callback( error, result ) so then we can wrap the calls on promises and other libraries which expect such well known standard.

"console.log" inside of libraries aren't a good way of dealing with problems since it polutes production logs and are "uncatchable", so we should use callback ( error ) instead and then "Catch" the error as i described on my previous comment.

Please let me know if this new patch solves your issues!

[jaggedsoft]

@kevflynn @programmrz @gnolmon I have just published v0.3.7 which rolls back the breaking changes

Version v0.4.0 will be coming soon, this includes some breaking changes. See: https://github.com/jaggedsoft/node-binance-api/pull/53/commits/23356221c2f2a830aa5afbd5bfa88a32962138f6

[kevflynn]

You guys are top notch - thanks @jaggedsoft and @hems I'll try to immerse myself in the code if you're looking to add contributors.

[jaggedsoft]

Resolved https://github.com/jaggedsoft/node-binance-api/commit/8bf3cde02379e20ba085d9460d9158487515d8d7