Skip to content
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

Read after write fails (Chrome DevTools Protocol server) #57

Closed
kenshaw opened this issue Apr 20, 2019 · 10 comments
Closed

Read after write fails (Chrome DevTools Protocol server) #57

kenshaw opened this issue Apr 20, 2019 · 10 comments

Comments

@kenshaw
Copy link

kenshaw commented Apr 20, 2019

Using this simple program:

package main

import (
	"context"
	"encoding/json"
	"errors"
	"flag"
	"log"
	"time"

	"nhooyr.io/websocket"
)

var flagRemote = flag.String("r", "", "remote url")

func main() {
	flag.Parse()
	if err := run(); err != nil {
		log.Fatalf("error: %v", err)
	}
}

func run() error {
	if *flagRemote == "" {
		return errors.New("must provide remote url via -r")
	}

	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
	defer cancel()

	c, _, err := websocket.Dial(ctx, *flagRemote)
	if err != nil {
		return err
	}
	defer c.Close(websocket.StatusInternalError, "")

	// write initial message
	w, err := c.Write(ctx, websocket.MessageText)
	if err != nil {
		return err
	}
	if _, err = w.Write([]byte(`{"id":1,"method":"Target.getTargets","params":{}}`)); err != nil {
		return err
	}
	if err = w.Close(); err != nil {
		return err
	}
	log.Printf("wrote Target.getTargets message")

	// read response
	jc := websocket.JSONConn{
		Conn: c,
	}
	var v interface{}
	err = jc.Read(ctx, &v)
	if err != nil {
		log.Fatalf("failed to read json: %v", err)
	}
	buf, err := json.Marshal(v)
	if err != nil {
		return err
	}
	log.Printf("received %s", string(buf))

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

And using it with Google Chrome's DevTools Protocol, on sending a simple message via this command:

Starting Chrome:

ken@ken-desktop:~/src/go/src/github.com/kenshaw/wstest$ google-chrome --headless --remote-debugging-port=0 --disable-gpu

DevTools listening on ws://127.0.0.1:32831/devtools/browser/4f7e048b-1c84-4f05-a7fe-39fd624fb07e

And then running the simple Go test program above:

ken@ken-desktop:~/src/go/src/github.com/kenshaw/wstest$ go build && ./wstest -r 'ws://127.0.0.1:32831/devtools/browser/4f7e048b-1c84-4f05-a7fe-39fd624fb07e'
2019/04/20 20:10:49 wrote Target.getTargets message
2019/04/20 20:10:49 failed to read json: failed to read json: websocket: failed to read message: websocket: connection broken: failed to read header: EOF
ken@ken-desktop:~/src/go/src/github.com/kenshaw/wstest$

Expected output: the response from Chrome, looking something like the following:

{"id":1,"result":{"targetInfos":[{"targetId":"D18C239D31BAB5964945BD536DD9C987","type":"page","title":"","url":"about:blank","attached":false,"browserContextId":"FECF894940A790B796F6A86E836A8242"}]}}

Note: both the gobwas/ws and gorilla/websocket packages work with Chrome's DevTools Protocol.

I've attempted to look through the nhooyr.io/websocket package to determine what the cause is, but have not been able to track down what the issue is. My guess is that it's a non-standard header or some such that Chrome is returning.

Perhaps I'm not using the package properly, if so, I apologize in advance, however I believe I correctly followed the examples provided in the package. Appreciate any pointers to doing this properly -- I'm also willing to help debug this issue. Thanks in advance!

@kenshaw
Copy link
Author

kenshaw commented Apr 20, 2019

You can try this with the chromedp project:

$ cd $GOPATH/src && mkdir -p github.com/chromedp && cd github.com/chromedp
$ git clone -b test-websocket-impls https://github.com/kenshaw/chromedp.git
$ cd $GOPATH/src/github.com/chromedp/chromedp
$ go test -v -tags gorilla
$ go test -v -tags gobwas
$ go test -v -tags nhooyr

@nhooyr
Copy link
Owner

nhooyr commented Apr 20, 2019

Thank you for the detailed report.

You seem to be using the package fine. Will fix the redundant and verbose errors in #47

I'm looking into this.

@nhooyr
Copy link
Owner

nhooyr commented Apr 20, 2019

So I've gotten to the bottom of this. It looks like a bug in chrome's dp tooling to me.

It cannot handle continuation frames. With gorilla/websocket and gobwas/ws, you're sending a single text frame but with this library, first the text frame is sent, then a continuation frame that finishes the frame. This is just due to the streaming style API, it doesn't know when to finish the message until you call w.Close.

@nhooyr
Copy link
Owner

nhooyr commented Apr 20, 2019

see

websocket/websocket_test.go

Lines 461 to 494 in d50ecee

func TestChrome(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
defer cancel()
c, _, err := websocket.Dial(ctx, "ws://127.0.0.1:56213/devtools/browser/f6427288-77aa-49d6-983f-a4afe482d5b6")
if err != nil {
t.Fatal(err)
}
defer c.Close(websocket.StatusInternalError, "")
// write initial message
err = c.WriteFrame(ctx, websocket.MessageText, []byte(`{"id":1,"method":"Target.getTargets","params":{}}`))
if err != nil {
t.Fatal(err)
}
log.Printf("wrote Target.getTargets message")
// read response
jc := websocket.JSONConn{
Conn: c,
}
var v interface{}
err = jc.Read(ctx, &v)
if err != nil {
log.Fatalf("failed to read json: %v", err)
}
buf, err := json.Marshal(v)
if err != nil {
t.Fatal(err)
}
log.Printf("received %s", string(buf))
c.Close(websocket.StatusNormalClosure, "")
}

I added a c.WriteFrame method to confirm this and now the code works. If you try and use the streaming API, it fails.

This is very surprising to me so I could be wrong, I'll check chrome's code.

@nhooyr
Copy link
Owner

nhooyr commented Apr 20, 2019

Known issue ChromeDevTools/devtools-protocol#24

@nhooyr
Copy link
Owner

nhooyr commented Apr 20, 2019

Filed a bug at https://bugs.chromium.org/p/chromium/issues/detail?id=954778

Thanks again for the report @kenshaw

@nhooyr
Copy link
Owner

nhooyr commented Apr 20, 2019

Going to close this as it's not a bug in this library and I do not want to expose a function because of a bug in chrome. I'll keep that branch for now in case you want to use it.

@nhooyr nhooyr closed this as completed Apr 20, 2019
@kenshaw
Copy link
Author

kenshaw commented Apr 20, 2019

While the Chrome Websocket implementation may not support this, it would be prudent for this package to do so, as the other websocket implementations available do. I agree it's not a bug in this package, but if you'd like greater adoption of this package, then it needs to support existing software in the wild. I imagine other websocket implementations also have this issue.

@nhooyr
Copy link
Owner

nhooyr commented Apr 20, 2019

You make a solid point. Given I already have the code for this, I think its ok to expose a method that performs a write of a byte slice directly instead of returning a writecloser.

@nhooyr
Copy link
Owner

nhooyr commented Apr 21, 2019

See #62

@nhooyr nhooyr closed this as completed Apr 21, 2019
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

2 participants