Skip to content

fix/fallback #520

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.

Sign up for GitHub

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

Merged
SuperCoolPencil merged 10 commits into from
Jun 29, 2026
Merged

fix/fallback #520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
08a8911
fix: broken fallback to single downloader
SuperCoolPencil Jun 26, 2026
fb6ee4c
fix: ensure ActiveWorkers is cleared after download completion
SuperCoolPencil Jun 26, 2026
5e3b584
fix: enhance debug logging for single-threaded downloads
SuperCoolPencil Jun 26, 2026
ea13f73
fix: add debug logging for Surge version and commit in global state i…
SuperCoolPencil Jun 26, 2026
c523a9f
fix: track pending bytes in progressReader for accurate chunk status …
SuperCoolPencil Jun 26, 2026
6d7d8c9
fix: implement context cancellation for single downloader requests
SuperCoolPencil Jun 26, 2026
b5135eb
'
SuperCoolPencil Jun 26, 2026
10ce64f
feat: single downloader Retry-After
SuperCoolPencil Jun 26, 2026
3b0fcad
feat: single downloader Retry-After
SuperCoolPencil Jun 26, 2026
f22d48b
Merge branch 'main' into fix/fallback
SuperCoolPencil Jun 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions internal/core/local_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ func (s *LocalDownloadService) reportProgressLoop() {
Speed: currentSpeed,
Elapsed: totalElapsed,
ActiveConnections: int(connections),
RateLimited: cfg.State.RateLimited.Load(),
}

// Chunk snapshots are expensive due to bitmap/progress copies.
Expand Down
1 change: 1 addition & 0 deletions internal/engine/events/events.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ type ProgressMsg struct {
BitmapWidth int
ActualChunkSize int64
ChunkProgress []int64
RateLimited bool
}

// DownloadCompleteMsg signals that the download finished successfully
Expand Down
6 changes: 6 additions & 0 deletions internal/engine/single/downloader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,17 @@ func (d *SingleDownloader) Download(ctx context.Context, rawurl, destPath string
ra = 5 * time.Second
}
utils.Debug("Single downloader: rate limited (%d), waiting %v (retry %d/%d)", resp.StatusCode, ra, rlRetries, maxRlRetries)
if d.State != nil {
d.State.RateLimited.Store(true)
}
select {
case <-dlCtx.Done():
return dlCtx.Err()
case <-time.After(ra):
}
if d.State != nil {
d.State.RateLimited.Store(false)
}
continue
Comment on lines 147 to 155

@greptile-apps greptile-apps Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 RateLimited flag not cleared on context cancellation

When the parent context is cancelled (e.g., user pauses or aborts) while the downloader is waiting out the rate-limit backoff, dlCtx.Done() fires and the function returns dlCtx.Err() at line 149 — skipping the RateLimited.Store(false) call at lines 152-154. The flag remains true in the shared ProgressState, so the TUI will continue showing "Rate limited, retrying…" even after the download has stopped. Adding defer d.State.RateLimited.Store(false) alongside the existing defer d.State.ActiveWorkers.Store(0) block (around line 86) would clean this up unconditionally on function exit.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: internal/engine/single/downloader.go
Line: 147-155

Comment:
**`RateLimited` flag not cleared on context cancellation**

When the parent context is cancelled (e.g., user pauses or aborts) while the downloader is waiting out the rate-limit backoff, `dlCtx.Done()` fires and the function returns `dlCtx.Err()` at line 149 — skipping the `RateLimited.Store(false)` call at lines 152-154. The flag remains `true` in the shared `ProgressState`, so the TUI will continue showing "Rate limited, retrying…" even after the download has stopped. Adding `defer d.State.RateLimited.Store(false)` alongside the existing `defer d.State.ActiveWorkers.Store(0)` block (around line 86) would clean this up unconditionally on function exit.

How can I resolve this? If you propose a fix, please make it concise.

}

Expand Down
2 changes: 2 additions & 0 deletions internal/engine/types/progress.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type ProgressState struct {
Error atomic.Pointer[error]
Paused atomic.Bool
Pausing atomic.Bool // Intermediate state: Pause requested but workers not yet exited
RateLimited atomic.Bool // Set when the downloader is backing off due to HTTP 429/rate-limit
cancelFunc context.CancelFunc

VerifiedProgress atomic.Int64 // Verified bytes written to disk (for UI progress)
Expand Down Expand Up @@ -251,6 +252,7 @@ func (ps *ProgressState) SessionReset() {
ps.Done.Store(false)
ps.Paused.Store(false)
ps.Pausing.Store(false)
ps.RateLimited.Store(false)
ps.Error.Store(nil)

// Clear mirrors error status
Expand Down
7 changes: 4 additions & 3 deletions internal/tui/model.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,10 @@ type DownloadModel struct {
done bool
started bool // Engine has confirmed start
err error
paused bool
pausing bool // UI state: transitioning to pause
resuming bool // UI state: waiting for async resume
paused bool
pausing bool // UI state: transitioning to pause
resuming bool // UI state: waiting for async resume
rateLimited bool
}

type RootModel struct {
Expand Down
1 change: 1 addition & 0 deletions internal/tui/process.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ func (m *RootModel) processProgressMsg(msg events.ProgressMsg) tea.Cmd {
d.Speed = msg.Speed
d.Elapsed = msg.Elapsed
d.Connections = msg.ActiveConnections
d.rateLimited = msg.RateLimited

// Keep "Resuming..." visible until we observe actual transfer.
if d.resuming && (d.Speed > 0 || d.Downloaded > prevDownloaded) {
Expand Down
3 changes: 3 additions & 0 deletions internal/tui/view.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,9 @@ func renderFocusedDetails(d *DownloadModel, w int, spinnerView string) string {
} else if d.resuming {
speedStr = "Resuming..."
etaStr = "..."
} else if d.rateLimited {
speedStr = "Rate limited, retrying..."
etaStr = "..."
} else if d.paused || d.Speed == 0 {
speedStr = "Paused"
if d.RateLimitSet && d.RateLimit > 0 {
Expand Down
Loading