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

sendMessageToTarget with large payloads causes "context canceled" #395

Closed
tmm1 opened this issue Jun 18, 2019 · 10 comments
Closed

sendMessageToTarget with large payloads causes "context canceled" #395

tmm1 opened this issue Jun 18, 2019 · 10 comments

Comments

@tmm1
Copy link
Contributor

tmm1 commented Jun 18, 2019

What versions are you running?

$ go list -m github.com/chromedp/chromedp
github.com/chromedp/chromedp v0.3.0
$ google-chrome --version
Google Chrome 75.0.3770.100
$ go version
go version go1.12.4 darwin/amd64

What did you do? Include clear steps.

I'm trying to implement a simple cookie jar between chromedp runs.

At the end of a session I use network.GetAllCookies to save a list of []*network.Cookie

At the beginning of a new session, I first convert my []*network.Cookie to []*network.CookieParam, then call network.SetCookies.

What did you expect to see?

I expect network.SetCookies to return no error.

What did you see instead?

For 8 cookies and 12 cookies, things work as expected.

But when I had 20 cookies, network.SetCookies is returning context canceled

@mvdan
Copy link
Contributor

mvdan commented Jun 18, 2019

Can you provide a way to reproduce the error? Without that, it's hard to tell what's actually going on.

Also see the FAQ in the readme; one common way to get unexpected context canceled errors is because Chrome unexpectedly died and the websocket connection was lost. So looking at what happened with your chrome process could help. We will probably give better errors in the future, but for now it shouldn't be too hard to manually debug.

@tmm1
Copy link
Contributor Author

tmm1 commented Jun 21, 2019

Also see the FAQ in the readme; one common way to get unexpected context canceled errors is because Chrome unexpectedly died and the websocket connection was lost.

Thanks for this pointer. It does seem like chrome is crashing, but I can't tell why.

I tried using chromedp.WithDebugf(log.Printf) but all I see is the last Target.sendMessageToTarget that triggers the crash. Is there some way to make chrome more verbose about what it dislikes about my message?

@tmm1
Copy link
Contributor Author

tmm1 commented Jun 21, 2019

I tried using this to enable logs, but I still get no logs to stderr or in the user-data-dir on my mac:

opts = append(opts,
    chromedp.Flag("enable-logging", "stderr"),
    chromedp.Flag("log-level", "0"),
    chromedp.Flag("v", "1"),
)

However, I was able to confirm my hunch that this is related to the size of the message sent to chrome. Here is a trivial reproduction:

chromedp.ActionFunc(func(ctx context.Context) error {
	_, _, err := runtime.Evaluate(fmt.Sprintf("//%s", strings.Repeat("x", 4096))).Do(ctx)
	return err
}),

@tmm1
Copy link
Contributor Author

tmm1 commented Jun 21, 2019

Interestingly: on v0.2.1-0.20190430114454-d15a83b92825 I don't see the crash, but on v0.3.0 I do. It also happens on v0.3.1-0.20190619195644-fd957a4d2901

@tmm1
Copy link
Contributor Author

tmm1 commented Jun 21, 2019

Works fine: runtime.Evaluate(fmt.Sprintf("//%s", strings.Repeat("x", 3892)))
Crashes: runtime.Evaluate(fmt.Sprintf("//%s", strings.Repeat("x", 3893)))

@tmm1 tmm1 changed the title network.SetCookies failing with "context canceled" with 20 cookies sendMessageToTarget with large payloads causes "context canceled" Jun 21, 2019
@mvdan
Copy link
Contributor

mvdan commented Jun 23, 2019

Interesting, thanks. 0.3.0 had plenty of internal refactors, so perhaps we introduced a regression.

If you'd like to help us fix this issue quickly, you could try bisecting which commit introduced the regression.

@mvdan mvdan removed the needs info label Jun 23, 2019
@tmm1
Copy link
Contributor Author

tmm1 commented Jun 24, 2019

The regression is in dc09186

@mvdan
Copy link
Contributor

mvdan commented Jun 25, 2019

Thanks! Will investigate.

@mvdan
Copy link
Contributor

mvdan commented Jun 26, 2019

The underlying problem is that Chrome doesn't accept fragmented websocket messages. See ChromeDevTools/devtools-protocol#24. They do support somewhat large messages, of up to 100MiB, without any fragmentation.

Annoyingly enough, when Chrome receives a fragmented message, it simply exits and drops the TCP websocket connection. Nothing useful is printed to its stderr, either.

You were seeing failures starting at around 3800 bytes, because we have a write buffer size of 4096 set up with our websocket writer. Your message went over that with its JSON encoding and header, so it got chunked.

We used to use a different websocket library in the past. Both implement fragmentation; the difference is that with the old library we set up a write buffer of 25MiB, and with the new one we set up a buffer of just 4KiB.

I'm going to "fix" this by simply increasing the size of the buffer. Chrome supports 100MiB, but I think using a buffer that large is overkill, and will increase the memory usage for everyone. I'll raise it to something more reasonable like 1MiB.

I've also raised this with the Chrome team again, in the hopes that they'll end up implementing fragmentation, or at least not having Chrome crash silently: ChromeDevTools/devtools-protocol#175

@mvdan mvdan closed this as completed in 0157080 Jun 26, 2019
mvdan added a commit that referenced this issue Jun 26, 2019
See the comment on wsWriteBufferSize for context. This somewhat fixes
the regression in dc09186, as we went from a 25MiB message size limit
to a 4KiB one. 1MiB seems like a reasonable middle ground for the time
being, keeping the memory usage low.

Fixes #395.
@tmm1
Copy link
Contributor Author

tmm1 commented Jun 29, 2019

Thanks a ton for tracking this down! Appreciate the work-around, as well as the upstream bug report for a better long term fix.

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