search

cavaliergopher / grab

A download manager package for Go
BSD 3-Clause "New" or "Revised" License
1.39k stars 150 forks source link

Grab hangs when server sends unexpected TCP RST packet #48

Open bts-dido opened 5 years ago

bts-dido commented 5 years ago

I'm trying to download larger files using grab. Sometimes it works as expected, but sometimes it's hanging. After debugging, it seems server sends unexpected TCP RST packet when downloading and causing grab to hang. Is it possible to catch TCP RST packet inside grab.(*transfer).copy ?

I know, setting timeout on http client will solve hanging issue, but it's not desirable, since we don't know how long will it take to download file. Also if server sends TCP RST, there's no point waiting for timeout, it would be nice if grab performs fail-fast for us.

Download failed: net/http: request canceled (Client.Timeout exceeded while reading body)

I'm able to simulate server sending unexpected TCP RST packets, so happy to lending a hand if debugging is needed.

Here's the stacktrace when grab is hanging. For ease of read, newlines are inserted between frames.

goroutine 22 [IO wait, 6 minutes]: internal/poll.runtime_pollWait /usr/lib/go-1.10/src/runtime/netpoll.go:173 +0x57

internal/poll.(*pollDesc).wait /usr/lib/go-1.10/src/internal/poll/fd_poll_runtime.go:85 +0x9b

internal/poll.(*pollDesc).waitRead /usr/lib/go-1.10/src/internal/poll/fd_poll_runtime.go:90 +0x3d

internal/poll.(*FD).Read /usr/lib/go-1.10/src/internal/poll/fd_unix.go:157 +0x17d

net.(*netFD).Read /usr/lib/go-1.10/src/net/fd_unix.go:202 +0x4f

net.(*conn).Read /usr/lib/go-1.10/src/net/net.go:176 +0x6a

crypto/tls.(*block).readFromUntil /usr/lib/go-1.10/src/crypto/tls/conn.go:493 +0x96

crypto/tls.(*Conn).readRecord /usr/lib/go-1.10/src/crypto/tls/conn.go:640 +0x1fa

crypto/tls.(*Conn).Read /usr/lib/go-1.10/src/crypto/tls/conn.go:1156 +0x100

net/http.(*persistConn).Read /usr/lib/go-1.10/src/net/http/transport.go:1453 +0x136

bufio.(*Reader).Read /usr/lib/go-1.10/src/bufio/bufio.go:202 +0x12c

io.(*LimitedReader).Read /usr/lib/go-1.10/src/io/io.go:446 +0x63

net/http.(*body).readLocked /usr/lib/go-1.10/src/net/http/transfer.go:778 +0x61

net/http.(*body).Read /usr/lib/go-1.10/src/net/http/transfer.go:770 +0xdd

net/http.(*bodyEOFSignal).Read /usr/lib/go-1.10/src/net/http/transport.go:2187 +0xdc

github.com/cavaliercoder/grab.(*transfer).copy /home/bat/go/src/github.com/cavaliercoder/grab/transfer.go:48 +0xf0

github.com/cavaliercoder/grab.(*Client).copyFile /home/bat/go/src/github.com/cavaliercoder/grab/client.go:457 +0x93

github.com/cavaliercoder/grab.(*Client).(github.com/cavaliercoder/grab.copyFile)-fm /home/bat/go/src/github.com/cavaliercoder/grab/client.go:88 +0x34

github.com/cavaliercoder/grab.(*Client).run /home/bat/go/src/github.com/cavaliercoder/grab/client.go:175 +0x6e

created by github.com/cavaliercoder/grab.(*Client).Do /home/bat/go/src/github.com/cavaliercoder/grab/client.go:88 +0x229

cavaliercoder commented 5 years ago

Apologies for late response on this. I'm hoping to solve this ASAP.

I'm able to simulate server sending unexpected TCP RST packets, so happy to lending a hand if debugging is needed.

This would be super helpful, yes please! Do you have a gist or article somewhere to demonstrate this?

cavaliercoder commented 5 years ago

I'm struggling to replicate this issue. It seems sending a random RST is harder than hoped. I'm looking at sending raw socket data which seems a step too far. Do you have a code sample?

As I look into this problem, I'm forming the opinion that handling resets should be covered by Go's HTTP reader returning an error: https://golang.org/src/net/http/response.go?#L54

    // The response body is streamed on demand as the Body field
    // is read. If the network connection fails or the server
    // terminates the response, Body.Read calls return an error.

If your program is hanging, I suspect there is a more subtle bug in my error handling or the http package.

cavaliercoder commented 5 years ago

Last commit at the time of stack trace: https://github.com/cavaliercoder/grab/tree/2c8601de6d9ac140430fd4c9de28509a0b855af5

And line 48 in transfer.go:

nr, er := c.r.Read(c.b)
bts-dido commented 5 years ago

@cavaliercoder Sorry for late response. Let me test again with latest commit and get back to you with the result.

I'm using iptables to simulate TCP RST packet.