hirokisan / bybit

Bybit client library for Go
https://pkg.go.dev/github.com/hirokisan/bybit/v2
MIT License
90 stars 58 forks source link

fix: update v5 wss public ping heartbeat 2 #127

Closed josephyim224 closed 1 year ago

josephyim224 commented 1 year ago

Surprised to find myself creating an update this soon 😢

After opening the websocket for some time, I am getting read tcp (my ip)->13.35.7.96:443: i/o timeout error for every ping message sent. I tried adding the original PingMessage and the connection stabilised afterwards.

While comparing another bybit library written in Python, it seems they are sending both PingMessage and ping TextMessage as well.

ref: https://github.com/bybit-exchange/pybit/blob/9f3e32d4d771727b6e966cfd748b241ed48a1f6a/pybit/_websocket_stream.py#L236

Can you kindly have a look? Thanks.

hirokisan commented 1 year ago

@josephyim224

After opening the websocket for some time

It would be helpful if you could share the code that can be reproduced at hand.

hirokisan commented 1 year ago

ref: https://github.com/gorilla/websocket/blob/76ecc29eff79f0cedf70c530605e486fc32131d1/conn.go#L64-L78

// TextMessage denotes a text data message. The text message payload is
// interpreted as UTF-8 encoded text data.
TextMessage = 1

// PingMessage denotes a ping control message. The optional message payload
// is UTF-8 encoded text.
PingMessage = 9

First, use PingMessage for ping. As a result, a Pong should be returned from the server.

Then, set {"op": "ping"} in messageBody as described below.

ref: https://bybit-exchange.github.io/docs/v5/ws/connect#how-to-send-the-heartbeat-packet

hirokisan commented 1 year ago

https://github.com/hirokisan/bybit/blob/d64f39670e34c798e4c75c0e4390282a0835cf39/v5_ws_public.go#L133

It seems that the connection was broken because SetReadDeadline was done first, but SetPongHandler was not called because the pong was not returned during the ping.

Because of this https://github.com/hirokisan/bybit/pull/127#issuecomment-1537332331 .

hirokisan commented 1 year ago

📝 By the way, I ran this code to check the behavior.

package main

import (
    "context"
    "log"

    "github.com/hirokisan/bybit/v2"
)

func main() {
    if err := run(); err != nil {
        log.Fatal(err)
    }
}

func run() error {
    wsClient := bybit.NewWebsocketClient()
    svc, err := wsClient.V5().Public(bybit.CategoryV5Linear)
    if err != nil {
        return err
    }
    if _, err := svc.SubscribeOrderBook(
        bybit.V5WebsocketPublicOrderBookParamKey{
            Depth:  500,
            Symbol: bybit.SymbolV5("ADAUSDT"),
        },
        func(response bybit.V5WebsocketPublicOrderBookResponse) error {
            return nil
        },
    ); err != nil {
        return err
    }

    if err := svc.Start(context.Background(), func(isWebsocketClosed bool, err error) {
        if isWebsocketClosed {
            log.Println("closed: ", err)
        }
    }); err != nil {
        return err
    }
    return nil
}
diff --git a/v5_ws_public.go b/v5_ws_public.go
index b182b36..1b2224c 100644
--- a/v5_ws_public.go
+++ b/v5_ws_public.go
@@ -130,8 +130,10 @@ func (s *V5WebsocketPublicService) Start(ctx context.Context, errHandler ErrHand
        go func() {
                defer close(done)
                defer s.connection.Close()
+
                _ = s.connection.SetReadDeadline(time.Now().Add(60 * time.Second))
-               s.connection.SetPongHandler(func(string) error {
+               s.connection.SetPongHandler(func(message string) error {
+                       log.Println("pong called")
                        _ = s.connection.SetReadDeadline(time.Now().Add(60 * time.Second))
                        return nil
                })
josephyim224 commented 1 year ago

Hi @hirokisan many thanks for your prompt reply and your fix is much more concise than I could have imagined!

I ran the sample code given and it worked flawlessly. Though strangely once I changed the subscription to spot market and kline the websocket issue starts to appear again. Please find below the modified sample:

package main

import (
    "context"
    "log"
    "time"

    "github.com/hirokisan/bybit/v2"
)

func main() {
    if err := run(); err != nil {
        log.Fatal(err)
    }
}

func run() error {
    wsClient := bybit.NewWebsocketClient()
    svc, err := wsClient.V5().Public(bybit.CategoryV5Spot) // changed to spot
    if err != nil {
        return err
    }

    // changed to kline subscription
    if _, err := svc.SubscribeKline(
        bybit.V5WebsocketPublicKlineParamKey{
            Symbol:   bybit.SymbolV5("BTCUSDT"),
            Interval: bybit.Interval120,
        }, func(vwpkr bybit.V5WebsocketPublicKlineResponse) error {
            // log timestamp indicate the client is still receiving message from exchange
            log.Println(time.Now().UTC().Format(time.RFC3339))
            return nil
        },
    ); err != nil {
        return nil
    }

    if err := svc.Start(context.Background(), func(isWebsocketClosed bool, err error) {
        log.Println("isWebsocketClosed: ", isWebsocketClosed)
        if err != nil {
            log.Println("error: ", err)
        }
    }); err != nil {
        return err
    }
    return nil
}

I also added log.Println("ping") to line 237 of v5_ws_public.go to ensure the ping function being used is the updated one and it has indeed been used. I reran the code multiple times and it always took ~5 minutes to fail.

Console log produced:

2023/05/07 23:12:18 program start
2023/05/07 23:12:19 2023-05-07T15:12:19Z
2023/05/07 23:12:22 2023-05-07T15:12:22Z
2023/05/07 23:12:25 2023-05-07T15:12:25Z
2023/05/07 23:12:30 2023-05-07T15:12:30Z
2023/05/07 23:12:39 ping
2023/05/07 23:12:40 2023-05-07T15:12:40Z
2023/05/07 23:12:42 2023-05-07T15:12:42Z
2023/05/07 23:12:43 2023-05-07T15:12:43Z
2023/05/07 23:12:47 2023-05-07T15:12:47Z
2023/05/07 23:12:54 2023-05-07T15:12:54Z
2023/05/07 23:12:55 2023-05-07T15:12:55Z
2023/05/07 23:12:58 2023-05-07T15:12:58Z
2023/05/07 23:12:59 ping
2023/05/07 23:13:02 2023-05-07T15:13:02Z
(... similar logs omitted ...)
2023/05/07 23:17:31 2023-05-07T15:17:31Z
2023/05/07 23:17:33 [err handler] isWebsocketClosed: false error: websocket: close 1006 (abnormal closure): unexpected EOF
2023/05/07 23:17:33 program end

However, if I split the ping message into two, the websocket connection won't be disconnected:

func (s *V5WebsocketPublicService) Ping() error {
    log.Println("ping")
    if err := s.writeMessage(websocket.PingMessage, nil); err != nil {
        return err
    }
    if err := s.writeMessage(websocket.TextMessage, []byte(`{"op":"ping"}`)); err != nil {
        return err
    }
    return nil
}

My wild guess is that Bybit seems to have different websocket server implementation for different market. For spot market it expects one ping message and one op code ping message, whereas linear market is not enforcing this?

Anyway really thanks for your time and hope you can kindly have a look at this as well. Cheers~

hirokisan commented 1 year ago

@josephyim224

For spot market it expects one ping message and one op code ping message

It appears so.

I would like to investigate the cause properly, but once I do, I will apply your suggestion.

I hope you will feel free to commit at any time. Thank you so much!

https://github.com/hirokisan/bybit/commit/931bcfeb98ba93175e01eac38b8414f9abb85991