jaggedsoft / node-binance-api

Node Binance API is an asynchronous node.js library for the Binance API designed to be easy to use.
MIT License
1.58k stars 768 forks source link

v0.6.0 Feature Discussion #125

Closed jaggedsoft closed 5 years ago

jaggedsoft commented 6 years ago

I am planning a complete rebuild of this project. Much has changed since it began, and we might as well make things easier. What are your thoughts?

Should we consider TypeScript? All other ideas are welcome

bitcoinvsalts commented 6 years ago

async/await Promises +1

Thanks Jon, you re the Man

nickreese commented 6 years ago

Jon — I gotta agree, you are the man. Thanks for putting together this amazing library.

My number one request: -- Tests for the bid book. I've seen 2 instances where my bid book has been out of sync since I've worked with the library. That said, can't recreate it. I'm 100% sure it isn't a sort issue... best I can tell it was a missed websocket update or something. It has been infrequent but is very concerning as it reduces faith in the cache. I believe the new websocket code forces the cache to be destroyed and rebuilt when a timeout occurs (can you verify that?)... but I think we should be checking to make sure we don't miss updates.

Here are some random thoughts: -- I've been testing the singleton code that was a community solution for a few weeks. It is rock solid. -- Promises would be great in many ways depending on how it was implemented. -- I'd like to see an error logging object. Logs for logs, errors for errors. -- The rate limiting code I submitted is still running strong with no issues. -- I could see a "market" order function that takes either buy or sell an argument. Right now I've got my own helper function to handle this but I could see it being useful for others. -- I'd like to see a console message if I place an order and it is in testing mode.

If you were getting ambitious: -- A system for mocking sever responses on testing mode. The "null, {}" is understandable and works, just could see this being really useful. -- In this same vein, mocking a websocket update on a testing trade would be epic as well. -- Built in filter checking (stepSize, orderSize, minNotional) could save some people headaches and a learning curve as well.

Overall, happy to help however I can.

Thank you for being a rockstar.

Edit: here is the depth cache ID monitoring ticket: https://github.com/binance-exchange/node-binance-api/issues/21. I haven't gotten a chance to test it.

alteredorange commented 6 years ago

Indeed, great work on this! For the most part things work really well. I'm more of a beginner, so most of my issues have been with documentation. I've worked out things like filled orders for last 24 hours, total buys and total sells over last 24 hours, etc. There's also the issue of websockets only updating once every second, but I think that's because Binance is only pushing data every second, not because of your code.

Thanks again for the awesome work!

bkrypt commented 6 years ago

Yup, excellent library, thanks Jon!

I am also very keen on Promises for async/await support. In my project I've actually wrapped this library up to be async/await friendly since the rest of the project uses it heavily. I also agree with @nickreese that logging needs a big overhaul to provide more useful, friendly info.

Also on @nickreese point of the depth cache, I've also noticed random occasions where an update is missed (usually, or at least most noticeably, a 0 quantity one to remove a price level), and the order book is completely invalidated, even more so when the symbol is trading under/over the broken level, depending on whether you're looking at ask/bid.

I did spend some time trying to fix this, and one attempt did actually include the same as suggested in binance-exchange#21 now that I see it - which I agree is a step in the right direction, but can confirm it doesn't solve the issue, but at least signals early that something has indeed gone wrong. I unfortunately couldn't spend more time on it then, so I fell back to the <symbol>@depth<level> streams (not currently available in the library). I may have some time tonight though (using another idea I've thought of since) to try solidify the depthCache updates.

I can also confirm that a timeout/reconnect on the socket does recreate the depth cache at the moment.

I'm a big advocate for TypeScript, being a C family programmer, but I have no problem keeping the library Javascript, a .d.ts could just be kept up to date with the library for those using the lib in a TypeScript project. I would however suggest maybe breaking parts of the code out into modules though, ie. http focused functionality, socket focused functionality, utility focused, etc.

nickreese commented 6 years ago

One note regarding my rate limiting code. It isn't designed to account for multiple instances of Binance (via the new object proposed above). Just something to keep in mind since rate limits are by IP, not by API key.

jaggedsoft commented 6 years ago

Thanks for the feedback and all the help, you guys are awesome. @nickreese I believe keith1024 has fixed the order book sync issues: https://github.com/jaggedsoft/node-binance-api/commit/39dd6ede8fb6b3044e18fa73fa007e54cc63ab0e

Time is an issue for me right now, but we'll keep making progress. Lots of improvements to come. Thanks for everything guys!!

jaggedsoft commented 6 years ago

So far the best solutions that I have found are bluebird + request-promise https://www.npmjs.com/package/request-promise .. 2.6 million downloads last month https://github.com/petkaantonov/bluebird .. 21 million downloads last month

They seem to be the easiest, and performance wise bluebird is faster than native promises

I prefer async/await to .then()

const Promise = require('bluebird');
const fs = Promise.promisifyAll(require('fs'));
let contents = await fs.readFileAsync('file.txt', 'utf-8');
bkrypt commented 6 years ago

Another HTTP library I can recommend that supports Promises out of the box is axios https://www.npmjs.com/package/axios

jaggedsoft commented 6 years ago

Haven't used it yet so it will take some getting used to, but it looks great

let contents = await axios.get(url);

axios.get(url)
  .catch(error => console.error(error))
  .then(response => console.log(response))
// not sure if this is good practice
let contents = await axios.get(url);
if ( contents instanceOf Error ) return contents;
bkrypt commented 6 years ago

I am 100% with you. I way prefer async/await vs then()/catch().

async/await lets you handle errors (rejected promises) like this, (the direction you were headed with the second snippet above).

try {
    const foo = await axios.get(url1);
    const bar = await axios.get(url2);
} catch (err) {
    // log and/or handle errors from foo or bar
    console.log(err);
}
bitcoinvsalts commented 6 years ago

Hi @jaggedsoft and everybody. How is everything going?

jaggedsoft commented 6 years ago

Great here, extremely busy the next few weeks then going to make a big push to get the new version out!

fngrz commented 6 years ago

Everyone has said this, but once more, with feeling... PROMISES! Multiple API instances in one app would be amazing.

jaggedsoft commented 6 years ago

There is a version coming with promise or async/await support for all REST requests. Can't figure out how to make it work on Websocket yet, but I guess that's fine

amitdahan commented 6 years ago

Maybe for sockets, the new Async Iterators feature fits? 👍

jaggedsoft commented 6 years ago

@amitdahan Any links or examples? I'm old school, so not as in touch with all the latest and greatest way of doing things as I'd like to be.. just now got in the habit of using async/await and love it

amitdahan commented 6 years ago

@jaggedsoft https://www.bignerdranch.com/blog/asyncing-feeling-about-javascript-generators/ https://medium.com/netscape/async-iterators-these-promises-are-killing-my-performance-4767df03d85b https://jakearchibald.com/2017/async-iterators-and-generators/

Can't say I messed with them myself, I'll create a gist/PR if I have the time!

Mahmoud-Masri commented 6 years ago

typescript +1

dmzoneill commented 6 years ago

Notifications

config

{
    "apikey" : "XYZ"
    ....
    "notify": [ 
        {
            "type": "libnotify"          
        },
        {
           "type": "irc",
           "server": "xyz.com",
           "port": "5678",
           "channel": "jaggedsoft"           
        }
     ]
}
jaggedsoft commented 6 years ago

Recommended way of people doing notifications for their own apps should be using discord.js because that also supports embeds which will be infinitely useful. This method also adds very little complexity image

fngrz commented 6 years ago

You can also use Telegraf.js with Telegram. http://telegraf.js.org/