Igosuki / binance-rs-async

Async client for the Binance APIs
https://crates.io/crates/binance-rs-async
Other
184 stars 113 forks source link

Some WebSocket payload doesn't have "e" tag. #13

Closed ghost closed 3 years ago

ghost commented 3 years ago

Example: Spot bookTicker

use code:

async fn book_ticker() {
    let keep_running = AtomicBool::new(true);
    let agg_trade: String = "btcusdt@bookTicker".to_string();

    let mut web_socket: WebSockets<'_, WebsocketEvent> =
        WebSockets::new(|events: WebsocketEvent| {
            if let WebsocketEvent::BookTicker(tick_event) = events {
                println!("{:?}", tick_event)
            }

            Ok(())
        });

    web_socket.connect(&agg_trade).await.unwrap(); // check error
    if let Err(e) = web_socket.event_loop(&keep_running).await {
        println!("Error: {}", e);
    }
    web_socket.disconnect().await.unwrap();
    println!("disconnected");
}

to sub will get error: Error: missing fieldeat line 1 column 107 disconnected

The reason for this is that Websocket Variant uses the tag Marco to assume that each event has an event_type tag. We should disable the tag macro and implement every struct that matches the doc.

Igosuki commented 3 years ago

This what WebsocketEventAlt and StreamEvent are for originally. You already have the receive the channel in 'StreamEvent' when you parse the event. As you can see, there is another stream event you missed which is the partial depth stream that yields an OrderBook

Igosuki commented 3 years ago

I agree that having a single data type is superior, but this is at the cost of additional bytes that add no direct value since the information is already encoded in the enum type, let me try to hack a bit at your PR to see if we can get rid of this problem.

ghost commented 3 years ago

Yes, I didn't notice the StreamEvent and WebsocketEventAlt type. so I think the original implementation is fine.

Igosuki commented 3 years ago

Well, I think you are right design wise, but performance wise the original one will be slightly nimbler. However you made a good point so I will improve upon your PR and add a few methods in the websockets impl to add combined streams as well.