Skip to content

Pinging not working as expected #186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pmwebster opened this issue Feb 11, 2020 · 6 comments
Closed

Pinging not working as expected #186

pmwebster opened this issue Feb 11, 2020 · 6 comments

Comments

@pmwebster
Copy link

pmwebster commented Feb 11, 2020

It's very possible I'm using this wrong, but pulling from your echo example, and your tests, a simple ping test with the following fails:

package main

import (
	"context"
	"fmt"
	"log"
	"net"
	"net/http"
	"time"

	"nhooyr.io/websocket"
	"nhooyr.io/websocket/wsjson"
)

func main() {
	l, err := net.Listen("tcp", "localhost:0")
	if err != nil {
		log.Fatalf("failed to listen: %v", err)
	}
	defer l.Close()

	s := &http.Server{
		Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
			err := pingServer(w, r)
			if err != nil {
				log.Printf("echo server: %v", err)
			}
		}),
		ReadTimeout:  time.Second * 15,
		WriteTimeout: time.Second * 15,
	}
	defer s.Close()

	go func() {
		err := s.Serve(l)
		if err != http.ErrServerClosed {
			log.Fatalf("failed to listen and serve: %v", err)
		}
	}()

	err = client("ws://" + l.Addr().String())
	if err != nil {
		log.Fatalf("client failed: %v", err)
	}
}

func pingServer(w http.ResponseWriter, r *http.Request) error {
	c, err := websocket.Accept(w, r, nil)
	if err != nil {
		return err
	}
	defer c.Close(websocket.StatusInternalError, "the sky is falling")

	log.Println("Pinging client")
	ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
	defer cancel()

	err = c.Ping(ctx)
	if err != nil {
		return err
	}
	return nil
}

func client(url string) error {
	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
	defer cancel()

	c, _, err := websocket.Dial(ctx, url, nil)
	if err != nil {
		return err
	}
	defer c.Close(websocket.StatusInternalError, "the sky is falling")

	for {
		v := map[string]int{}
		err = wsjson.Read(ctx, c, &v)
		if err != nil {
			return err
		}

		fmt.Printf("received: %v\n", v)
	}

	c.Close(websocket.StatusNormalClosure, "")
	return nil
}

Output:

2020/02/10 22:09:42 Pinging client
2020/02/10 22:09:47 echo server: failed to ping: failed to wait for pong: context deadline exceeded
2020/02/10 22:09:47 client failed: failed to read json: failed to get reader: failed to read frame header: read tcp 127.0.0.1:51279->127.0.0.1:51278: read: connection reset by peer
exit status 1

Is there a different way this is supposed to be used? Thanks for any insight!

@nhooyr
Copy link
Contributor

nhooyr commented Feb 11, 2020

So you send a ping but because you are never reading in the server goroutine, the pong is never read.

See the docs on the Ping method.

@nhooyr nhooyr closed this as completed Feb 11, 2020
@nhooyr
Copy link
Contributor

nhooyr commented Feb 11, 2020

@nhooyr
Copy link
Contributor

nhooyr commented Feb 11, 2020

If you explicitly want to ignore messages, use c.CloseRead and then your example will work.

@nhooyr
Copy link
Contributor

nhooyr commented Feb 11, 2020

package main

import (
	"context"
	"fmt"
	"log"
	"net"
	"net/http"
	"time"

	"nhooyr.io/websocket"
	"nhooyr.io/websocket/wsjson"
)

func main() {
	l, err := net.Listen("tcp", "localhost:0")
	if err != nil {
		log.Fatalf("failed to listen: %v", err)
	}
	defer l.Close()

	s := &http.Server{
		Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
			err := pingServer(w, r)
			if err != nil {
				log.Printf("echo server: %v", err)
			}
		}),
		ReadTimeout:  time.Second * 15,
		WriteTimeout: time.Second * 15,
	}
	defer s.Close()

	go func() {
		err := s.Serve(l)
		if err != http.ErrServerClosed {
			log.Fatalf("failed to listen and serve: %v", err)
		}
	}()

	err = client("ws://" + l.Addr().String())
	if err != nil {
		log.Fatalf("client failed: %v", err)
	}
}

func pingServer(w http.ResponseWriter, r *http.Request) error {
	c, err := websocket.Accept(w, r, nil)
	if err != nil {
		return err
	}
	defer c.Close(websocket.StatusInternalError, "the sky is falling")

	log.Println("Pinging client")
	ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
	defer cancel()

	ctx = c.CloseRead(ctx)

	err = c.Ping(ctx)
	if err != nil {
		return err
	}
	return nil
}

func pong(ctx context.Context, c *websocket.Conn) {
	ctx, cancel := context.WithTimeout(ctx, time.Second*10)
	defer cancel()
}

func client(url string) error {
	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
	defer cancel()

	c, _, err := websocket.Dial(ctx, url, nil)
	if err != nil {
		return err
	}
	defer c.Close(websocket.StatusInternalError, "the sky is falling")

	for {
		v := map[string]int{}
		err = wsjson.Read(ctx, c, &v)
		if err != nil {
			return err
		}

		fmt.Printf("received: %v\n", v)
	}

	c.Close(websocket.StatusNormalClosure, "")
	return nil
}

This succeeds with:

$ go run main.go
2020/02/10 23:19:38 Pinging client
2020/02/10 23:19:38 client failed: failed to read json: failed to get reader: failed to read frame header: EOF

Now only the client fails.

@pmwebster
Copy link
Author

Thanks for clarifying that! The docs make perfect sense now. Cheers!

@csrar
Copy link

csrar commented Dec 11, 2020

I'm not sure if did it wright but because I have spend a day trying to make work the ping functionality I'm leaving an example how I'm using it

package main

import (
	"context"
	"fmt"
	"log"
	"net/http"
	"time"

	uuid "github.com/satori/go.uuid"
	"nhooyr.io/websocket"
	"nhooyr.io/websocket/wsjson"
)

func startWSserver(connectionList map[string]*websocket.Conn) {
	fn := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		fmt.Println(connectionList)

		c, err := websocket.Accept(w, r, &websocket.AcceptOptions{
			Subprotocols: []string{},
		})
		fmt.Println(c.Subprotocol())
		if err != nil {
			log.Println("ss", err)
			return
		}
		defer c.Close(websocket.StatusInternalError, "Session closed by the server")

		u1 := uuid.NewV4()
		connectionList[u1.String()] = c

		ctx, cancel := context.WithCancel(context.Background())
		defer cancel()

		// go Ping(c)
		t := time.NewTicker(time.Second * 2)
		defer t.Stop()

		for {
			select {
			case <-ctx.Done():
				c.Close(websocket.StatusNormalClosure, "")
				return
			case <-t.C:
				err = wsjson.Write(ctx, c, "hi")
				if err != nil {
					log.Println(err)
					return
				}
			}
		}
	})
	go http.ListenAndServe("10.10.40.25:4123", fn)
}
// Ping is
func Ping(c *websocket.Conn, agent string, sessionList map[string]*websocket.Conn) {

	go func() {
		ctxRead := context.Background()
		c.CloseRead(ctxRead)
	}()
	go func() {
		ctxPing, cancel := context.WithTimeout(context.Background(), time.Second*10)
		defer cancel()
		pingErr := c.Ping(ctxPing)
		if pingErr == nil {
			fmt.Println("Pinged client")

		} else {
			fmt.Println("Failed to ping client")
			fmt.Println(pingErr)
			fmt.Println("removing session")
			delete(sessionList, agent)
		}
	}()
	time.Sleep(time.Second * 5)
}
func main() {
	connectionList := map[string]*websocket.Conn{}
	startWSserver(connectionList)
	for {
		for key, conn := range connectionList {
			fmt.Println("->before ping")
			Ping(conn, key, connectionList)
			fmt.Println("->after ping ")
		}
		time.Sleep(time.Second * 300)
	}
}```

If anyone want to try it, just replace the address of the Web server. 
In my scenario I need to keep the socket connection as far the client still connected and I the client that I'm going to write would not send any data but pong reply to the server

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants