Skip to content

Gin compatabilty #166

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
nmec opened this issue Oct 25, 2019 · 21 comments · Fixed by #236
Closed

Gin compatabilty #166

nmec opened this issue Oct 25, 2019 · 21 comments · Fixed by #236

Comments

@nmec
Copy link

nmec commented Oct 25, 2019

I'm trying to integrate this into my existing app which is using the Gin framework, but I'm unable to establish a connection from the browser. Any ideas how I can get this working?

Server

package main

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

	"github.com/gin-gonic/gin"
	"nhooyr.io/websocket"
	"nhooyr.io/websocket/wsjson"
)

func main() {
	router := gin.New()

	router.GET("/ws", handler)

	router.Run(":7000")
}

func handler(c *gin.Context) {
	log.Println("upgrading")
	conn, wsErr := websocket.Accept(c.Writer, c.Request, &websocket.AcceptOptions{
		InsecureSkipVerify: true,
	})

	if wsErr != nil {
		c.AbortWithError(http.StatusInternalServerError, wsErr)
		return
	}

	defer conn.Close(websocket.StatusInternalError, "Closed unexepetedly")

	log.Println("ready")

	ctx, cancel := context.WithTimeout(c.Request.Context(), time.Minute)
	defer cancel()
	ctx = conn.CloseRead(ctx)

	tick := time.NewTicker(time.Second * 5)
	defer tick.Stop()

	for {
		select {
		case <-ctx.Done():
			log.Println("done")
			conn.Close(websocket.StatusNormalClosure, "")
			return
		case <-tick.C:
			writeErr := wsjson.Write(ctx, conn, map[string]interface{}{
				"msg": "hello",
			})
			if writeErr != nil {
				log.Println(writeErr)
				return
			}
		}
	}
}

Client

const ws = new WebSocket('ws://localhost:7000/ws');
ws.onopen = console.log;
ws.onmessage = console.log;

JS Error
WebSocket connection to 'ws://localhost:7000/ws' failed: Error during WebSocket handshake: net::ERR_INVALID_HTTP_RESPONSE

Server logs

[GIN-debug] [WARNING] Running in "debug" mode. Switch to "release" mode in production.
 - using env:   export GIN_MODE=release
 - using code:  gin.SetMode(gin.ReleaseMode)

[GIN-debug] GET    /ws                       --> main.handler (1 handlers)
[GIN-debug] Listening and serving HTTP on :7000
2019/10/25 15:17:49 upgrading
2019/10/25 15:17:49 ready
2019/10/25 15:17:54 done
@nhooyr
Copy link
Contributor

nhooyr commented Oct 25, 2019

What does the Go program log?

@nhooyr
Copy link
Contributor

nhooyr commented Oct 25, 2019

Also, print wsErr.

@nmec
Copy link
Author

nmec commented Oct 25, 2019

Thanks for the speedy reply - I've updated the description with the server logs.

@nhooyr
Copy link
Contributor

nhooyr commented Oct 25, 2019

Mhmm. I believe the issue is that gin doesn't allow writing a response before hijacking the connection the way net/http does. Gorilla works because it hijacks first, then writes the handshake response directly to the tcp connection. We could do this as well but it'd be janky.

What do you get in the network tab of the browser? What does the HTTP response look like?

@nmec
Copy link
Author

nmec commented Oct 25, 2019

HTTP Request:
image
HTTP Response:
image

@nhooyr
Copy link
Contributor

nhooyr commented Oct 25, 2019

Got it, what does the headers tab look like for the response?

@nmec
Copy link
Author

nmec commented Oct 25, 2019

No response headers are shown, only the request.

@nhooyr
Copy link
Contributor

nhooyr commented Oct 25, 2019

Cool, lines up with my reasoning.

Looks like gin deviates from net/http for some reason. Have to call an additional WriteHeaderNow method to write the header.

https://github.com/gin-gonic/gin/blob/094f9a9105f8e7b971a01a64677eef5a1f7bcd9b/response_writer.go#L69

Opened gin-gonic/gin#2108

Will fix tonight.

@nmec
Copy link
Author

nmec commented Oct 25, 2019

Amazing, thanks so much for the quick responses.

@nmec
Copy link
Author

nmec commented Oct 25, 2019

Not sure if this is of use to you to fix the issue, but I just tried gobwas/ws for the heck of it and it works perfectly:

package main

import (
	"encoding/json"
	"log"
	"time"

	"github.com/gin-gonic/gin"
	"github.com/gobwas/ws"
	"github.com/gobwas/ws/wsutil"
)

func main() {
	router := gin.Default()

	router.GET("/ws", handler)

	router.Run(":7000")
}

func handler(c *gin.Context) {
	log.Println("upgrading")
	conn, _, _, err := ws.UpgradeHTTP(c.Request, c.Writer)
	if err != nil {
		log.Println(err)
	}

	log.Println("ready")

	go func() {
		tick := time.NewTicker(time.Second * 5)
		defer tick.Stop()
		defer conn.Close()

		for {
			select {
			case <-tick.C:
				log.Println("tick")
				data, _ := json.Marshal(map[string]interface{}{
					"msg": "hello",
				})

				err = wsutil.WriteServerMessage(conn, ws.OpText, data)
				if err != nil {
					log.Println(err)
				}

			}
		}
	}()
}

@nhooyr
Copy link
Contributor

nhooyr commented Oct 25, 2019

If you add w.(interface{ WriteHeaderNow() }.WriteHeaderNow()) on this line https://github.com/nhooyr/websocket/blob/8bf88772e1209d96a00bc38276cda458661ebe2b/handshake.go#L143 things should work. Could you test it?

@nhooyr
Copy link
Contributor

nhooyr commented Oct 25, 2019

gohack may be helpful. https://github.com/rogpeppe/gohack

@nhooyr nhooyr mentioned this issue Oct 25, 2019
nhooyr added a commit that referenced this issue Oct 25, 2019
@nhooyr
Copy link
Contributor

nhooyr commented Oct 29, 2019

Best to fix this in gin by making the response writer Hijack method flush any written header.

@nhooyr nhooyr closed this as completed Oct 29, 2019
@nhooyr
Copy link
Contributor

nhooyr commented Nov 1, 2019

See gin-gonic/gin#2120

@elnappo
Copy link

elnappo commented May 7, 2020

Did I miss the oblivious solution here? Not sure how to fix that. Do I really need to use gohack, because this would be a very ugly way? Why did you close your pull request in gin-gonic/gin?

@nmec
Copy link
Author

nmec commented May 7, 2020

In the end I ended up using gobwas/ws over this library, since my implementation shared above worked with Gin whilst this was awaiting a fix upstream. It's a little bit lower level than this library, but with the util package it's not too much extra work and you get a better understanding of how the underlaying protocol is working. Sorry if that's not much help for your immediate problem!

@nhooyr
Copy link
Contributor

nhooyr commented May 7, 2020

There are a ton of unmerged PRs and issues on gin. I'd recommend against using it and I closed that PR because it didn't seem to have a chance of getting merged.

However, given it's prevalence in the ecosystem, I'll add a patch here to make it work :)

@nhooyr nhooyr reopened this May 7, 2020
@nhooyr
Copy link
Contributor

nhooyr commented May 7, 2020

Yea my bad I meant to add a fix here but I guess it slipped.

See gin-gonic/gin#2108

nhooyr added a commit that referenced this issue May 10, 2020
nhooyr added a commit that referenced this issue May 10, 2020
nhooyr added a commit that referenced this issue May 10, 2020
nhooyr added a commit that referenced this issue May 10, 2020
nhooyr added a commit that referenced this issue May 10, 2020
@nhooyr
Copy link
Contributor

nhooyr commented May 10, 2020

Master works now on Gin, will tag a release EOD :)

@elnappo
Copy link

elnappo commented May 11, 2020

Thank you! :)

@nhooyr
Copy link
Contributor

nhooyr commented May 18, 2020

Sorry for the delay, tagged v1.8.6 today!

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

Successfully merging a pull request may close this issue.

3 participants