resilient network disconnect#540
Conversation
…tion, state snapshotting, and automatic retry logic
…-related download failures
Binary Size Analysis
|
❌ Test Failures on
|
❌ Test Failures on
|
❌ Test Failures on
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| remainingTasks := queue.DrainRemaining() | ||
| remainingTasks = append(remainingTasks, activeRemaining...) |
There was a problem hiding this comment.
When a worker exhausts its retries, it removes its task from activeTasks before returning the error, and that task is no longer in the queue. This snapshot therefore omits the range that caused the failure, overstates Downloaded, and can resume without ever downloading the missing bytes.
Rule Used: What: All code changes must include tests for edge... (source)
Artifacts
Repro: focused Go test driving worker failure, persistence, state reload, and resume readback
- Contains supporting evidence from the run (text/x-go; charset=utf-8).
Repro: verbose failing test output with serialized DownloadRecord and missing-range readback
- Keeps the command output available without making the summary code-heavy.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/strategy/concurrent/downloader.go
Line: 671-672
Comment:
**Failed Range Missing From Snapshot**
When a worker exhausts its retries, it removes its task from `activeTasks` before returning the error, and that task is no longer in the queue. This snapshot therefore omits the range that caused the failure, overstates `Downloaded`, and can resume without ever downloading the missing bytes.
**Rule Used:** What: All code changes must include tests for edge... ([source](https://app.greptile.com/surge-org-3/-/custom-context?memory=2b22782d-3452-4d55-b059-e631b2540ce8))
How can I resolve this? If you propose a fix, please make it concise.| isNetworkErr := errors.Is(err, types.ErrNetworkFailure) | ||
| canRetryGeneric := !isNetworkErr && qt.retries < 3 |
There was a problem hiding this comment.
When RunDownload returns ErrNetworkFailure, this condition excludes it from the scheduler's only requeue branch, while networkRetries is never used elsewhere. The task is removed from active tracking without being queued or marked errored, so an exhausted network retry silently leaves the saved download orphaned.
Rule Used: What: All code changes must include tests for edge... (source)
Artifacts
Repro: focused Go scheduler harness that drives the ErrNetworkFailure branch
- Contains supporting evidence from the run (text/x-go; charset=utf-8).
Repro: verbose focused test output and failure trace
- Keeps the command output available without making the summary code-heavy.
Repro: concise scheduler state and progress-event capture
- Keeps the command output available without making the summary code-heavy.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/scheduler/scheduler.go
Line: 717-718
Comment:
**Network Failure Skips Requeue**
When `RunDownload` returns `ErrNetworkFailure`, this condition excludes it from the scheduler's only requeue branch, while `networkRetries` is never used elsewhere. The task is removed from active tracking without being queued or marked errored, so an exhausted network retry silently leaves the saved download orphaned.
**Rule Used:** What: All code changes must include tests for edge... ([source](https://app.greptile.com/surge-org-3/-/custom-context?memory=2b22782d-3452-4d55-b059-e631b2540ce8))
How can I resolve this? If you propose a fix, please make it concise.| if errors.As(err, &opErr) { | ||
| return true | ||
| } |
There was a problem hiding this comment.
Every net.OpError is classified as transient, including one wrapping a permanent DNS result such as an unknown host. A download with a misspelled or removed hostname therefore enters the ten-attempt connectivity loop instead of failing normally, delaying the actionable error for several minutes.
Rule Used: What: All code changes must include tests for edge... (source)
Artifacts
- Contains supporting evidence from the run (text/x-go; charset=utf-8).
- Keeps the command output available without making the summary code-heavy.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/strategy/concurrent/network_errors.go
Line: 53-55
Comment:
**Permanent DNS Errors Become Transient**
Every `net.OpError` is classified as transient, including one wrapping a permanent DNS result such as an unknown host. A download with a misspelled or removed hostname therefore enters the ten-attempt connectivity loop instead of failing normally, delaying the actionable error for several minutes.
**Rule Used:** What: All code changes must include tests for edge... ([source](https://app.greptile.com/surge-org-3/-/custom-context?memory=2b22782d-3452-4d55-b059-e631b2540ce8))
How can I resolve this? If you propose a fix, please make it concise.1 participant
Greptile Summary
This PR adds recovery behavior for transient network failures. The main changes are:
Confidence Score: 2/5
The recovery path can persist an incomplete task list and then remove the download without requeuing it.
internal/strategy/concurrent/downloader.go, internal/scheduler/scheduler.go, and internal/strategy/concurrent/network_errors.go
What T-Rex did
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant W as Worker participant D as Downloader participant E as Event Worker participant S as Scheduler W->>W: Exhaust network retries W->>W: Remove failed task from activeTasks W-->>D: Return transient error D->>D: Build snapshot from queue and active tasks Note over D: Failed byte range is absent D->>E: Persist paused snapshot D-->>S: Return ErrNetworkFailure S->>S: Remove active download Note over S: Network error does not enter requeue branch%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant W as Worker participant D as Downloader participant E as Event Worker participant S as Scheduler W->>W: Exhaust network retries W->>W: Remove failed task from activeTasks W-->>D: Return transient error D->>D: Build snapshot from queue and active tasks Note over D: Failed byte range is absent D->>E: Persist paused snapshot D-->>S: Return ErrNetworkFailure S->>S: Remove active download Note over S: Network error does not enter requeue branchPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "refactor: remove network retry logic and..." | Re-trigger Greptile