Skip to content

Fix issue #190: Implement robust retry logic for downloads - #203

Merged
fangfufu merged 3 commits into
masterfrom
fix-issue-190-retry-logic
May 10, 2026
Merged

Fix issue #190: Implement robust retry logic for downloads#203
fangfufu merged 3 commits into
masterfrom
fix-issue-190-retry-logic

Conversation

@fangfufu

@fangfufu fangfufu commented May 9, 2026

Copy link
Copy Markdown
Owner

This pull request addresses issue #190 where files could appear incomplete or corrupt when encountering HTTP 429 (Rate Limited) errors or network interruptions.

Technical Analysis

After analyzing the source code in src/link.c and src/network.c, I identified several root causes for the reported behavior:

  1. Missing Retry Logic in Link_download: The function responsible for reading file content (via range requests) lacked a retry mechanism. Upon encountering an HTTP 429 error or a truncated download, it logged an error but proceeded with whatever partial data it had received.
  2. Improper Error Handling: If an HTTP error occurred, Link_download would still attempt to call memmove on the data buffer using the negative error code (e.g., -ENOENT) as the size. This could lead to memory corruption or crashes, and certainly resulted in incorrect read returns.
  3. Blocking Network Sleep: The 429 handling in network.c used a blocking sleep() while holding the global transfer_lock. This effectively froze the entire filesystem's network activity for all threads without actually retrying the failed request.
  4. Cumulative Data Bug: The retry loop in Link_download_full did not reset the transfer buffer between attempts. Since the write callback appends data, a retry after a 429 would prepended the error page's HTML to the actual file content.

Fixes Applied

  • Robust Retry Loop: Link_download now includes a do-while loop that handles both HTTP temporary failures and partial downloads by retrying until the full range is successfully retrieved.
  • Safe Error Paths: Ensured Link_download returns immediately on unrecoverable errors, avoiding invalid memory operations on the output buffer.
  • Buffer Resets: Updated Link_download_full to clear its data buffer before each retry attempt to prevent data corruption.
  • Resource Cleanup: Fixed a resource leak in Link_download_cleanup by ensuring the curl handle is always cleaned up, even on error paths.
  • Decoupled Retries: Moved the retry delay from the core network processing to individual request handlers, allowing other concurrent transfers to proceed uninterrupted.

These improvements make httpdirfs much more resilient to aggressive server-side rate limiting and unstable network connections.

@codacy-production

codacy-production Bot commented May 9, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity · 0 duplication

Metric Results
Complexity 0
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

This commit addresses the issue where files could appear incomplete or
corrupt when encountering HTTP 429 errors or network interruptions.

Key changes:
- Added a retry loop to Link_download for range requests.
- Fixed a bug in Link_download_full where data would accumulate on retries.
- Resolved a resource leak and potential crash in Link_download_cleanup.
- Moved blocking sleep from network.c to individual request handlers.
@fangfufu
fangfufu force-pushed the fix-issue-190-retry-logic branch from c98cbe5 to 393bef8 Compare May 9, 2026 23:57
@sonarqubecloud

Copy link
Copy Markdown

@fangfufu
fangfufu merged commit 08a7987 into master May 10, 2026
10 checks passed
@fangfufu
fangfufu deleted the fix-issue-190-retry-logic branch May 10, 2026 00:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant