httpdl/cli: drop --auto-split; classify errors by type, not message text
Two Pike cleanups. --auto-split was a third connection-count policy (beside -x and -s) that added an extra knob rather than keeping the model minimal. Removing it also retires the now-dead autoConns/maxAutoConns and the per-host transport cap that existed only to scale for it; min(split, M*-x) is the sole policy again. main's exit-code mapping fell back to substring-matching third-party error text (strings.Contains "connection refused"/"timeout"/...), which rots when a dependency rewords a message. The typed checks (errors.Is on the syscall errno, *net.DNSError, net.Error.Timeout, context.DeadlineExceeded) already cover the real stdlib errors -- verified end to end: refused -> 6, bad host -> 19. The two cases that genuinely needed the text match are our own errors, now typed sentinels: httpdl.ErrTimeout (idle timeout) and the bt metadata timeout wrapping context.DeadlineExceeded.
This commit is contained in:
@@ -18,9 +18,6 @@ Needs Go 1.25+.
|
|||||||
# HTTP download with 16 connections
|
# HTTP download with 16 connections
|
||||||
got -x16 -s16 https://example.com/big.iso
|
got -x16 -s16 https://example.com/big.iso
|
||||||
|
|
||||||
# let got pick the connection count from the file size
|
|
||||||
got --auto-split https://example.com/big.iso
|
|
||||||
|
|
||||||
# resume an interrupted download
|
# resume an interrupted download
|
||||||
got -c https://example.com/big.iso
|
got -c https://example.com/big.iso
|
||||||
|
|
||||||
@@ -47,7 +44,6 @@ Run `got --help` (or `--help=all`) for every option.
|
|||||||
| `-c, --continue` | resume a partial download | false |
|
| `-c, --continue` | resume a partial download | false |
|
||||||
| `-x, --max-connection-per-server` | connections to one server (1–16) | 1 |
|
| `-x, --max-connection-per-server` | connections to one server (1–16) | 1 |
|
||||||
| `-s, --split` | split a download into N connections | 5 |
|
| `-s, --split` | split a download into N connections | 5 |
|
||||||
| `--auto-split` | choose connections from file size (overrides `-x`/`-s`) | false |
|
|
||||||
| `-j, --max-concurrent-downloads` | downloads at once | 5 |
|
| `-j, --max-concurrent-downloads` | downloads at once | 5 |
|
||||||
| `--max-overall-download-limit` | global speed cap | 0 (off) |
|
| `--max-overall-download-limit` | global speed cap | 0 (off) |
|
||||||
| `--checksum` | verify the finished file: `TYPE=DIGEST` (sha-256, sha-1, …) | |
|
| `--checksum` | verify the finished file: `TYPE=DIGEST` (sha-256, sha-1, …) | |
|
||||||
|
|||||||
4
bt/bt.go
4
bt/bt.go
@@ -392,7 +392,9 @@ func (d *Download) awaitInfo(ctx context.Context, t *torrent.Torrent) error {
|
|||||||
case <-ctx.Done():
|
case <-ctx.Done():
|
||||||
return ctx.Err()
|
return ctx.Err()
|
||||||
case <-deadline:
|
case <-deadline:
|
||||||
return fmt.Errorf("timed out fetching metadata after %s", d.opts.MetaTimeout)
|
// Wrap DeadlineExceeded so the exit-code mapping classifies it as a timeout
|
||||||
|
// (2) via errors.Is, rather than matching on the message text.
|
||||||
|
return fmt.Errorf("timed out fetching metadata after %s: %w", d.opts.MetaTimeout, context.DeadlineExceeded)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -81,7 +81,6 @@ var options = []Opt{
|
|||||||
{Long: "max-connection-per-server", Short: 'x', Kind: Int, Default: "1", Min: 1, Max: 16, Help: "max connections to one server (1-16)", Tag: Basic},
|
{Long: "max-connection-per-server", Short: 'x', Kind: Int, Default: "1", Min: 1, Max: 16, Help: "max connections to one server (1-16)", Tag: Basic},
|
||||||
{Long: "split", Short: 's', Kind: Int, Default: "5", Min: 1, Help: "split a download into N connections; actual connections are min(max-connection-per-server, split), and -x defaults to 1", Tag: Basic},
|
{Long: "split", Short: 's', Kind: Int, Default: "5", Min: 1, Help: "split a download into N connections; actual connections are min(max-connection-per-server, split), and -x defaults to 1", Tag: Basic},
|
||||||
{Long: "min-split-size", Short: 'k', Kind: Size, Default: "20M", Min: 1 << 20, Max: 1 << 30, Help: "do not split a piece smaller than SIZE (1M-1024M)", Tag: Basic},
|
{Long: "min-split-size", Short: 'k', Kind: Size, Default: "20M", Min: 1 << 20, Max: 1 << 30, Help: "do not split a piece smaller than SIZE (1M-1024M)", Tag: Basic},
|
||||||
{Long: "auto-split", Kind: Bool, Default: "false", Help: "got-only: pick the connection count from file size (one per min-split-size, up to 16); overrides -x/-s", Tag: HTTP},
|
|
||||||
{Long: "force-sequential", Short: 'Z', Kind: Bool, Default: "false", Help: "download each command-line URI as its own file instead of mirroring them", Tag: HTTP},
|
{Long: "force-sequential", Short: 'Z', Kind: Bool, Default: "false", Help: "download each command-line URI as its own file instead of mirroring them", Tag: HTTP},
|
||||||
{Long: "max-tries", Short: 'm', Kind: Int, Default: "5", Min: 0, Help: "max retries per segment (0 = unlimited)", Tag: HTTP},
|
{Long: "max-tries", Short: 'm', Kind: Int, Default: "5", Min: 0, Help: "max retries per segment (0 = unlimited)", Tag: HTTP},
|
||||||
{Long: "timeout", Short: 't', Kind: Int, Default: "60", Min: 1, Help: "connection timeout in seconds", Tag: HTTP},
|
{Long: "timeout", Short: 't', Kind: Int, Default: "60", Min: 1, Help: "connection timeout in seconds", Tag: HTTP},
|
||||||
|
|||||||
@@ -57,7 +57,6 @@ type Config struct {
|
|||||||
Split int // --split: total connections across all mirrors
|
Split int // --split: total connections across all mirrors
|
||||||
MaxConnPerServer int // --max-connection-per-server: per-host connection cap
|
MaxConnPerServer int // --max-connection-per-server: per-host connection cap
|
||||||
MinSplit int64
|
MinSplit int64
|
||||||
AutoSplit bool // --auto-split (got-only): pick the connection count from file size
|
|
||||||
Tries int // 0 = unlimited
|
Tries int // 0 = unlimited
|
||||||
Timeout time.Duration
|
Timeout time.Duration
|
||||||
FileAlloc string // none | prealloc | trunc | falloc
|
FileAlloc string // none | prealloc | trunc | falloc
|
||||||
@@ -120,7 +119,7 @@ type Config struct {
|
|||||||
// connections.
|
// connections.
|
||||||
type Download struct {
|
type Download struct {
|
||||||
uris []string // mirror list; uris[0] is the primary (naming + resume key)
|
uris []string // mirror list; uris[0] is the primary (naming + resume key)
|
||||||
maxConns int // manual segment-worker cap = min(split, len(uris)*max-connection-per-server); --auto-split derives its own count per file in segmented()
|
maxConns int // segment-worker cap = min(split, len(uris)*max-connection-per-server)
|
||||||
cfg Config
|
cfg Config
|
||||||
client *http.Client
|
client *http.Client
|
||||||
limit []*rate.Limiter
|
limit []*rate.Limiter
|
||||||
@@ -216,17 +215,10 @@ func New(uris []string, cfg Config) *Download {
|
|||||||
if perHost < 1 {
|
if perHost < 1 {
|
||||||
perHost = 1
|
perHost = 1
|
||||||
}
|
}
|
||||||
// The transport must allow as many concurrent connections per host as we may
|
|
||||||
// actually open. --auto-split scales up to maxAutoConns per host once the file
|
|
||||||
// size is known (in segmented), so raise the cap to that ceiling when it is on.
|
|
||||||
connCap := perHost
|
|
||||||
if cfg.AutoSplit {
|
|
||||||
connCap = maxAutoConns
|
|
||||||
}
|
|
||||||
tr := &http.Transport{
|
tr := &http.Transport{
|
||||||
Proxy: http.ProxyFromEnvironment,
|
Proxy: http.ProxyFromEnvironment,
|
||||||
MaxConnsPerHost: connCap,
|
MaxConnsPerHost: perHost,
|
||||||
MaxIdleConnsPerHost: connCap,
|
MaxIdleConnsPerHost: perHost,
|
||||||
ResponseHeaderTimeout: cfg.Timeout,
|
ResponseHeaderTimeout: cfg.Timeout,
|
||||||
TLSHandshakeTimeout: connectTimeout,
|
TLSHandshakeTimeout: connectTimeout,
|
||||||
DialContext: dialContext,
|
DialContext: dialContext,
|
||||||
@@ -671,13 +663,7 @@ func (d *Download) probe(ctx context.Context, uri string) (probeResult, error) {
|
|||||||
// segmented downloads total bytes over d.maxConns workers, with per-segment
|
// segmented downloads total bytes over d.maxConns workers, with per-segment
|
||||||
// resume from the control file.
|
// resume from the control file.
|
||||||
func (d *Download) segmented(ctx context.Context, out string, total int64, etag, lastmod string) error {
|
func (d *Download) segmented(ctx context.Context, out string, total int64, etag, lastmod string) error {
|
||||||
// --auto-split picks the connection count from the file size now that total is
|
segs := makeSegments(total, d.cfg.MinSplit, d.maxConns)
|
||||||
// known; otherwise use the manual cap fixed at construction.
|
|
||||||
conns := d.maxConns
|
|
||||||
if d.cfg.AutoSplit {
|
|
||||||
conns = autoConns(total, d.cfg.MinSplit)
|
|
||||||
}
|
|
||||||
segs := makeSegments(total, d.cfg.MinSplit, conns)
|
|
||||||
if c := d.resumeControl(out, etag, lastmod); c != nil {
|
if c := d.resumeControl(out, etag, lastmod); c != nil {
|
||||||
segs = segsFromControl(c)
|
segs = segsFromControl(c)
|
||||||
} else if d.cfg.Continue && !fileExists(controlPath(out)) {
|
} else if d.cfg.Continue && !fileExists(controlPath(out)) {
|
||||||
@@ -809,10 +795,11 @@ func (d *Download) fetchOnce(ctx context.Context, f *os.File, s *seg, uri string
|
|||||||
return d.pump(ctx, f, s, body)
|
return d.pump(ctx, f, s, body)
|
||||||
}
|
}
|
||||||
|
|
||||||
// errIdleTimeout marks a transfer that stalled past the idle window, so callers
|
// ErrTimeout marks a transfer that stalled past the idle window (--timeout): a
|
||||||
// can report a distinct "timed out" instead of the bare "context canceled" that
|
// distinct "timed out" instead of the bare "context canceled" the cancelled
|
||||||
// the cancelled request would otherwise surface (item [39]).
|
// request would otherwise surface (item [39]). main maps it to exit code 2 via
|
||||||
var errIdleTimeout = errors.New("idle timeout: no data received")
|
// errors.Is, so it must stay on the error chain (CONTRACT).
|
||||||
|
var ErrTimeout = errors.New("idle timeout: no data received")
|
||||||
|
|
||||||
// idleReader records the time of the last byte received; a background watcher
|
// idleReader records the time of the last byte received; a background watcher
|
||||||
// (idleGuard) cancels the request once a full idle window passes with no
|
// (idleGuard) cancels the request once a full idle window passes with no
|
||||||
@@ -834,7 +821,7 @@ func (ir *idleReader) Read(p []byte) (int, error) {
|
|||||||
if err != nil && ir.timedOut.Load() {
|
if err != nil && ir.timedOut.Load() {
|
||||||
// The cancellation came from our watcher, not the caller's ctx, so
|
// The cancellation came from our watcher, not the caller's ctx, so
|
||||||
// translate the read error into the distinct idle-timeout sentinel.
|
// translate the read error into the distinct idle-timeout sentinel.
|
||||||
return n, fmt.Errorf("%w (after %s)", errIdleTimeout, ir.idle)
|
return n, fmt.Errorf("%w (after %s)", ErrTimeout, ir.idle)
|
||||||
}
|
}
|
||||||
return n, err
|
return n, err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -55,27 +55,3 @@ func makeSegments(total, minSplit int64, conns int) []seg {
|
|||||||
}
|
}
|
||||||
return segs
|
return segs
|
||||||
}
|
}
|
||||||
|
|
||||||
// maxAutoConns is the ceiling --auto-split scales to. aria2 caps
|
|
||||||
// max-connection-per-server (-x) at 16, so a got-chosen count honours the same
|
|
||||||
// per-server limit rather than inventing a looser one.
|
|
||||||
const maxAutoConns = 16
|
|
||||||
|
|
||||||
// autoConns picks a connection count from the file size, used only when
|
|
||||||
// --auto-split is set: one connection per min-split-size of content, at least 1
|
|
||||||
// and at most maxAutoConns. It is the got-only stand-in for hand-tuning -x/-s,
|
|
||||||
// and because the count never exceeds total/minSplit, makeSegments yields exactly
|
|
||||||
// that many balanced segments.
|
|
||||||
func autoConns(total, minSplit int64) int {
|
|
||||||
if minSplit < 1 {
|
|
||||||
minSplit = 1
|
|
||||||
}
|
|
||||||
n := total / minSplit
|
|
||||||
if n < 1 {
|
|
||||||
n = 1
|
|
||||||
}
|
|
||||||
if n > maxAutoConns {
|
|
||||||
n = maxAutoConns
|
|
||||||
}
|
|
||||||
return int(n)
|
|
||||||
}
|
|
||||||
|
|||||||
@@ -47,31 +47,6 @@ func TestMakeSegments(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestAutoConns(t *testing.T) {
|
|
||||||
const m = int64(20 << 20) // 20 MiB min-split, aria2's default
|
|
||||||
tests := []struct {
|
|
||||||
name string
|
|
||||||
total int64
|
|
||||||
minSplit int64
|
|
||||||
want int
|
|
||||||
}{
|
|
||||||
{"under one min-split is a single connection", 5 << 20, m, 1},
|
|
||||||
{"exactly one min-split", m, m, 1},
|
|
||||||
{"five min-splits", 100 << 20, m, 5},
|
|
||||||
{"caps at maxAutoConns", 10 << 30, m, maxAutoConns},
|
|
||||||
{"rounds down to whole pieces", m*3 + 1, m, 3},
|
|
||||||
{"tiny min-split still capped", 1 << 30, 1 << 20, maxAutoConns},
|
|
||||||
{"zero total is one connection", 0, m, 1},
|
|
||||||
}
|
|
||||||
for _, tc := range tests {
|
|
||||||
t.Run(tc.name, func(t *testing.T) {
|
|
||||||
if got := autoConns(tc.total, tc.minSplit); got != tc.want {
|
|
||||||
t.Errorf("autoConns(%d, %d) = %d, want %d", tc.total, tc.minSplit, got, tc.want)
|
|
||||||
}
|
|
||||||
})
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestSegProgress(t *testing.T) {
|
func TestSegProgress(t *testing.T) {
|
||||||
s := seg{start: 100, end: 199} // length 100
|
s := seg{start: 100, end: 199} // length 100
|
||||||
if s.length() != 100 {
|
if s.length() != 100 {
|
||||||
|
|||||||
34
main.go
34
main.go
@@ -530,7 +530,6 @@ func httpConfig(opts *cli.Options, single bool, overallDL *rate.Limiter) httpdl.
|
|||||||
Split: opts.Int("split"),
|
Split: opts.Int("split"),
|
||||||
MaxConnPerServer: opts.Int("max-connection-per-server"),
|
MaxConnPerServer: opts.Int("max-connection-per-server"),
|
||||||
MinSplit: opts.Size("min-split-size"),
|
MinSplit: opts.Size("min-split-size"),
|
||||||
AutoSplit: opts.Bool("auto-split"),
|
|
||||||
Tries: opts.Int("max-tries"),
|
Tries: opts.Int("max-tries"),
|
||||||
Timeout: time.Duration(opts.Int("timeout")) * time.Second,
|
Timeout: time.Duration(opts.Int("timeout")) * time.Second,
|
||||||
FileAlloc: opts.Str("file-allocation"),
|
FileAlloc: opts.Str("file-allocation"),
|
||||||
@@ -717,42 +716,29 @@ func exitCode(err error) int {
|
|||||||
|
|
||||||
// isNameResolution reports whether err is (or wraps) a DNS lookup failure —
|
// isNameResolution reports whether err is (or wraps) a DNS lookup failure —
|
||||||
// "name resolution failed" (19), which usually points at the user's own
|
// "name resolution failed" (19), which usually points at the user's own
|
||||||
// network or resolver rather than a dead remote.
|
// network or resolver rather than a dead remote. The stdlib reports every such
|
||||||
|
// failure (no-such-host, server-misbehaving) as *net.DNSError.
|
||||||
func isNameResolution(err error) bool {
|
func isNameResolution(err error) bool {
|
||||||
var de *net.DNSError
|
var de *net.DNSError
|
||||||
if errors.As(err, &de) {
|
return errors.As(err, &de)
|
||||||
return true
|
|
||||||
}
|
|
||||||
msg := strings.ToLower(err.Error())
|
|
||||||
return strings.Contains(msg, "no such host") || strings.Contains(msg, "server misbehaving")
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// isNetwork reports whether err is a connection-level failure — refused, reset,
|
// isNetwork reports whether err is a connection-level failure — refused, reset,
|
||||||
// or an unreachable host/network — a "network problem" (6).
|
// or an unreachable host/network — a "network problem" (6). A dial error from
|
||||||
|
// net/http wraps the syscall errno, so the typed check sees through it.
|
||||||
func isNetwork(err error) bool {
|
func isNetwork(err error) bool {
|
||||||
if errors.Is(err, syscall.ECONNREFUSED) || errors.Is(err, syscall.ECONNRESET) ||
|
return errors.Is(err, syscall.ECONNREFUSED) || errors.Is(err, syscall.ECONNRESET) ||
|
||||||
errors.Is(err, syscall.ENETUNREACH) || errors.Is(err, syscall.EHOSTUNREACH) {
|
errors.Is(err, syscall.ENETUNREACH) || errors.Is(err, syscall.EHOSTUNREACH)
|
||||||
return true
|
|
||||||
}
|
|
||||||
msg := strings.ToLower(err.Error())
|
|
||||||
return strings.Contains(msg, "connection refused") ||
|
|
||||||
strings.Contains(msg, "connection reset") ||
|
|
||||||
strings.Contains(msg, "network is unreachable") ||
|
|
||||||
strings.Contains(msg, "no route to host")
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// isTimeout reports whether err is (or wraps) a timeout: a deadline exceeded, a
|
// isTimeout reports whether err is (or wraps) a timeout: a deadline exceeded, a
|
||||||
// net.Error that timed out, or an idle-read timeout reported as such.
|
// net.Error that timed out, or our own idle-read timeout (httpdl.ErrTimeout).
|
||||||
func isTimeout(err error) bool {
|
func isTimeout(err error) bool {
|
||||||
if errors.Is(err, context.DeadlineExceeded) {
|
if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, httpdl.ErrTimeout) {
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
var ne net.Error
|
var ne net.Error
|
||||||
if errors.As(err, &ne) && ne.Timeout() {
|
return errors.As(err, &ne) && ne.Timeout()
|
||||||
return true
|
|
||||||
}
|
|
||||||
msg := strings.ToLower(err.Error())
|
|
||||||
return strings.Contains(msg, "timeout") || strings.Contains(msg, "timed out")
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// saveSession writes the sources of every download that did not finish to path,
|
// saveSession writes the sources of every download that did not finish to path,
|
||||||
|
|||||||
@@ -69,16 +69,14 @@ func TestExitCode(t *testing.T) {
|
|||||||
err error
|
err error
|
||||||
want int
|
want int
|
||||||
}{
|
}{
|
||||||
{"idle timeout", errors.New("idle timeout: no data received"), 2},
|
{"idle timeout", fmt.Errorf("segment 0: %w (after 1m0s)", httpdl.ErrTimeout), 2},
|
||||||
{"metadata timed out", errors.New("timed out fetching metadata after 3s"), 2},
|
{"metadata timed out", fmt.Errorf("timed out fetching metadata after 3s: %w", context.DeadlineExceeded), 2},
|
||||||
{"deadline", context.DeadlineExceeded, 2},
|
{"deadline", context.DeadlineExceeded, 2},
|
||||||
{"file exists", fmt.Errorf("/tmp/x: %w (use --allow-overwrite or -c)", httpdl.ErrOutputExists), 13},
|
{"file exists", fmt.Errorf("/tmp/x: %w (use --allow-overwrite or -c)", httpdl.ErrOutputExists), 13},
|
||||||
{"dns typed", dnsErr, 19},
|
{"dns typed", dnsErr, 19},
|
||||||
{"dns wrapped through retries", fmt.Errorf("http://x: %w (after 5 tries)", dnsErr), 19},
|
{"dns wrapped through retries", fmt.Errorf("http://x: %w (after 5 tries)", dnsErr), 19},
|
||||||
{"dns by message", errors.New("dial tcp: lookup nope.invalid: no such host"), 19},
|
|
||||||
{"refused typed", refused, 6},
|
{"refused typed", refused, 6},
|
||||||
{"refused wrapped through retries", fmt.Errorf("http://x: %w (after 5 tries)", refused), 6},
|
{"refused wrapped through retries", fmt.Errorf("http://x: %w (after 5 tries)", refused), 6},
|
||||||
{"refused by message", errors.New("dial tcp 127.0.0.1:1: connect: connection refused"), 6},
|
|
||||||
{"not found", fmt.Errorf("%w: http://x/y: 404 Not Found", httpdl.ErrNotFound), 3},
|
{"not found", fmt.Errorf("%w: http://x/y: 404 Not Found", httpdl.ErrNotFound), 3},
|
||||||
{"too slow", fmt.Errorf("http://x: %w (<= %d bytes/sec)", httpdl.ErrTooSlow, 1024), 5},
|
{"too slow", fmt.Errorf("http://x: %w (<= %d bytes/sec)", httpdl.ErrTooSlow, 1024), 5},
|
||||||
{"checksum", fmt.Errorf("/tmp/x: %w (want a, got b)", httpdl.ErrChecksum), 32},
|
{"checksum", fmt.Errorf("/tmp/x: %w (want a, got b)", httpdl.ErrChecksum), 32},
|
||||||
@@ -227,7 +225,7 @@ func TestFollowUpInfoHashDedup(t *testing.T) {
|
|||||||
func TestReportExit(t *testing.T) {
|
func TestReportExit(t *testing.T) {
|
||||||
canceled := download.Result{Name: "a", Err: context.Canceled}
|
canceled := download.Result{Name: "a", Err: context.Canceled}
|
||||||
done := download.Result{Name: "b", Err: nil}
|
done := download.Result{Name: "b", Err: nil}
|
||||||
timeout := download.Result{Name: "c", Err: errors.New("idle timeout")}
|
timeout := download.Result{Name: "c", Err: httpdl.ErrTimeout}
|
||||||
|
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
|
|||||||
Reference in New Issue
Block a user