From 75e191f86bd53f83ead78f12b8042b301979a634 Mon Sep 17 00:00:00 2001 From: Deivid Soto Date: Sat, 30 May 2026 08:59:33 +0200 Subject: [PATCH] fix(docker): three streaming/reliability bugs found in live docker test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit funnel: urlPattern matched api.trycloudflare.com before the real quick-tunnel URL. Cloudflared logs the control-plane endpoint early, so the agent was advertising a dead URL. Tighten regex to require at least one hyphen — quick tunnels are always multi-word (e.g. make-appointments-negotiation-blacks). Covers with funnel_test.go regression test. download(oneshot): progress reporter called /api/internal/agent/status with a synthetic "oneshot-" task ID that is not a UUID, causing the server to return 400 every 5 s for the entire download. Pass nil client to NewProgressReporter for one-shot mode; flush/ReportFinal are no-ops when reporter == nil so terminal output continues unchanged. torrent: piece-completion SQLite DB (anacrolix) was created inside the download dir (DataDir). On NFS/SMB mounts SQLite file locking times out, emitting a warning and falling back to an ephemeral in-memory DB. Add PieceCompletionDir to TorrentConfig; the daemon now passes config.DataDir() (agent state dir, always local) so the DB stays off the network mount. One-shot download leaves the field empty → harmless in-memory fallback as before. --- internal/cmd/daemon.go | 19 ++++++++-------- internal/cmd/download.go | 9 ++++---- internal/engine/progress.go | 20 +++++++++++++++-- internal/engine/torrent.go | 25 +++++++++++++++++++-- internal/funnel/funnel.go | 10 ++++++--- internal/funnel/funnel_test.go | 40 ++++++++++++++++++++++++++++++++++ 6 files changed, 102 insertions(+), 21 deletions(-) create mode 100644 internal/funnel/funnel_test.go diff --git a/internal/cmd/daemon.go b/internal/cmd/daemon.go index 2e0c074..425cee0 100644 --- a/internal/cmd/daemon.go +++ b/internal/cmd/daemon.go @@ -265,15 +265,16 @@ func runDaemonStart() error { // Create torrent downloader torrentDl, err := engine.NewTorrentDownloader(engine.TorrentConfig{ - DataDir: cfg.Download.Dir, - MetadataTimeout: metaTimeout, - StallTimeout: stallTimeout, - MaxTimeout: 0, - MaxDownloadRate: maxDl, - MaxUploadRate: maxUl, - ListenPort: cfg.Download.ListenPort, - SeedEnabled: false, - VPNTunnel: vpnTunnel, + DataDir: cfg.Download.Dir, + PieceCompletionDir: config.DataDir(), // keep piece-completion DB off NFS/SMB mounts + MetadataTimeout: metaTimeout, + StallTimeout: stallTimeout, + MaxTimeout: 0, + MaxDownloadRate: maxDl, + MaxUploadRate: maxUl, + ListenPort: cfg.Download.ListenPort, + SeedEnabled: false, + VPNTunnel: vpnTunnel, }) if err != nil { return fmt.Errorf("create torrent downloader: %w", err) diff --git a/internal/cmd/download.go b/internal/cmd/download.go index bd5ceab..5bf31a5 100644 --- a/internal/cmd/download.go +++ b/internal/cmd/download.go @@ -119,11 +119,10 @@ func runDownloadWithDeps(input, method string, deps downloadDeps) error { return fmt.Errorf("create downloader: %w", err) } - // Create a dummy reporter (no API reporting for one-shot) - reporter := engine.NewProgressReporter( - deps.newAgentClient(cfg.Auth.APIURL, cfg.Auth.APIKey, "unarr/"+Version), - 5*time.Second, - ) + // Local-only reporter: one-shot downloads have no server-side task, so a nil + // client keeps terminal progress working without spamming the status API + // (which 400s the synthetic "oneshot-" id). + reporter := engine.NewProgressReporter(nil, 5*time.Second) debridDl := deps.newDebridDl() diff --git a/internal/engine/progress.go b/internal/engine/progress.go index eba8814..e5eefe0 100644 --- a/internal/engine/progress.go +++ b/internal/engine/progress.go @@ -45,10 +45,19 @@ type ProgressReporter struct { lastCheckAt time.Time // last time we reported for control-signal polling } -// NewProgressReporter creates a reporter that flushes every interval. +// NewProgressReporter creates a reporter that flushes every interval. A nil +// client yields a local-only reporter that tracks progress for terminal output +// but never calls the API — used by one-shot `unarr download`, which has no +// server-side task to report against (its synthetic "oneshot-" id is not a UUID +// and the /api/internal/agent/status endpoint 400s it). Passing the typed nil +// straight into the interface field would make it non-nil, so guard explicitly. func NewProgressReporter(ac *agent.Client, interval time.Duration) *ProgressReporter { + var rep StatusReporter + if ac != nil { + rep = ac + } return &ProgressReporter{ - reporter: ac, + reporter: rep, interval: interval, latest: make(map[string]*Task), lastReported: make(map[string]TaskStatus), @@ -108,6 +117,9 @@ func (r *ProgressReporter) Run(ctx context.Context) error { } func (r *ProgressReporter) flush(ctx context.Context) { + if r.reporter == nil { + return // local-only reporter (one-shot): nothing to send + } r.mu.Lock() tasks := make([]*Task, 0, len(r.latest)) for _, t := range r.latest { @@ -239,6 +251,10 @@ func (r *ProgressReporter) handleResponse(task *Task, resp *agent.StatusResponse // ReportFinal sends a final status update for a completed/failed task. func (r *ProgressReporter) ReportFinal(ctx context.Context, task *Task) { + if r.reporter == nil { + r.Untrack(task.ID) + return // local-only reporter (one-shot) + } update := task.ToStatusUpdate() if _, err := r.reporter.ReportStatus(ctx, update); err != nil { log.Printf("[%s] final report failed: %v", task.ID[:8], err) diff --git a/internal/engine/torrent.go b/internal/engine/torrent.go index 6a4e8eb..efcddbe 100644 --- a/internal/engine/torrent.go +++ b/internal/engine/torrent.go @@ -61,7 +61,12 @@ var defaultTrackers = []string{ // TorrentConfig holds settings for the BitTorrent downloader. type TorrentConfig struct { - DataDir string + DataDir string + // PieceCompletionDir, when non-empty, stores the piece-completion SQLite DB + // in this directory instead of DataDir. Use the agent's local state dir + // (not the download dir) so the DB never lands on NFS/SMB volumes where + // SQLite locking times out. + PieceCompletionDir string MetadataTimeout time.Duration // how long to wait for torrent metadata (default 15m, 0 = unlimited) StallTimeout time.Duration // no progress during download for this long = stall (default 10m) MaxTimeout time.Duration // absolute maximum per torrent (default 0 = unlimited) @@ -113,7 +118,23 @@ func NewTorrentDownloader(cfg TorrentConfig) (*TorrentDownloader, error) { // Storage: mmap instead of default file backend. // The library author notes file storage has "very high system overhead". // mmap improves I/O throughput and piece verification speed significantly. - tcfg.DefaultStorage = storage.NewMMap(cfg.DataDir) + // + // When PieceCompletionDir is set (daemon always passes the agent state dir), + // keep the piece-completion SQLite DB off the download dir so it never lands + // on NFS/SMB where SQLite's file locking times out and emits a warning. + if cfg.PieceCompletionDir != "" { + if mkErr := os.MkdirAll(cfg.PieceCompletionDir, 0o755); mkErr != nil { + log.Printf("[torrent] piece-completion dir create failed (%v), DB stays in download dir", mkErr) + tcfg.DefaultStorage = storage.NewMMap(cfg.DataDir) + } else if pc, pcErr := storage.NewDefaultPieceCompletionForDir(cfg.PieceCompletionDir); pcErr != nil { + log.Printf("[torrent] piece-completion db in %q failed (%v), falling back to download dir", cfg.PieceCompletionDir, pcErr) + tcfg.DefaultStorage = storage.NewMMap(cfg.DataDir) + } else { + tcfg.DefaultStorage = storage.NewMMapWithCompletion(cfg.DataDir, pc) + } + } else { + tcfg.DefaultStorage = storage.NewMMap(cfg.DataDir) + } // Fixed port for incoming peer connections (enables UPnP port mapping). // With ListenPort=0, only ~30% of peers can connect to us. diff --git a/internal/funnel/funnel.go b/internal/funnel/funnel.go index 6a8640a..7f1b76a 100644 --- a/internal/funnel/funnel.go +++ b/internal/funnel/funnel.go @@ -32,9 +32,13 @@ import ( ) // urlPattern matches the `https://.trycloudflare.com` URL cloudflared -// prints when a Quick Tunnel is registered. The hostname has a random -// hyphen-separated label followed by .trycloudflare.com. -var urlPattern = regexp.MustCompile(`https://[a-z0-9-]+\.trycloudflare\.com`) +// prints when a Quick Tunnel is registered. Quick Tunnel hostnames are always +// several hyphen-joined dictionary words (e.g. +// `make-appointments-negotiation-blacks`), so we require at least one hyphen. +// This deliberately excludes cloudflared's control-plane endpoint +// `https://api.trycloudflare.com`, which appears earlier in the log stream — a +// permissive `[a-z0-9-]+` matched `api` first and we advertised a dead URL. +var urlPattern = regexp.MustCompile(`https://[a-z0-9]+(?:-[a-z0-9]+)+\.trycloudflare\.com`) // Config controls how the tunnel is launched. type Config struct { diff --git a/internal/funnel/funnel_test.go b/internal/funnel/funnel_test.go new file mode 100644 index 0000000..fa9280d --- /dev/null +++ b/internal/funnel/funnel_test.go @@ -0,0 +1,40 @@ +package funnel + +import "testing" + +func TestURLPattern(t *testing.T) { + cases := []struct { + name string + line string + want string + }{ + { + name: "real quick tunnel banner", + line: "2026-05-29T22:18:33Z INF | https://make-appointments-negotiation-blacks.trycloudflare.com |", + want: "https://make-appointments-negotiation-blacks.trycloudflare.com", + }, + { + name: "two-word hostname", + line: "https://blue-river.trycloudflare.com is ready", + want: "https://blue-river.trycloudflare.com", + }, + { + name: "control-plane api endpoint is ignored", + line: `2026-05-29T22:17:59Z DBG POST https://api.trycloudflare.com/tunnel`, + want: "", + }, + { + name: "no trycloudflare url", + line: "2026-05-29T22:17:44Z INF Requesting new quick Tunnel on trycloudflare.com...", + want: "", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := urlPattern.FindString(tc.line); got != tc.want { + t.Fatalf("FindString(%q) = %q, want %q", tc.line, got, tc.want) + } + }) + } +}