Igosuki/binance-rs-async

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

Opened this issue · 4 comments

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 field e at 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.

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

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.

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

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.