Skip to content

fix: read too much data #2

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
wesraph merged 1 commit into from
Nov 30, 2025
Merged

fix: read too much data #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 8 additions & 2 deletions archive_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ func TestExtractZipFileBasic(t *testing.T) {
// Skip ZIP extraction test - requires TUI integration
t.Skip("ZIP extraction requires TUI program which can't be easily mocked")

d := NewDownloader(mock.URL(), 4, 1024, "", 0, false, 3, 0, 1024*1024*1024)
d, err := NewDownloader(mock.URL(), 4, 1024, "", 0, false, 3, 0, 1024*1024*1024)
if err != nil {
t.Fatalf("NewDownloader failed: %v", err)
}

// Create a mock TUI program
p := tea.NewProgram(initialModel(mock.URL(), 1024, false, false, 1))
Expand Down Expand Up @@ -105,7 +108,10 @@ func TestExtractZipFileLarge(t *testing.T) {
mock := NewMockHTTPServer(content)
defer mock.Close()

d := NewDownloader(mock.URL(), 4, 1024, "", 0, false, 3, 0, 1024*1024*1024)
d, err := NewDownloader(mock.URL(), 4, 1024, "", 0, false, 3, 0, 1024*1024*1024)
if err != nil {
t.Fatalf("NewDownloader failed: %v", err)
}
p := tea.NewProgram(initialModel(mock.URL(), 1024, false, false, 1))

// Extract - this would fail with file descriptor exhaustion
Expand Down
105 changes: 75 additions & 30 deletions downloader_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,12 @@ func TestGetFileSize(t *testing.T) {
mock := NewMockHTTPServer(content)
defer mock.Close()

d := NewDownloader(mock.URL(), 4, 1024*1024, "", 0, false, 3, 100*time.Millisecond, 1024*1024*1024)
d, err := NewDownloader(mock.URL(), 4, 1024*1024, "", 0, false, 3, 100*time.Millisecond, 1024*1024*1024)
if err != nil {
t.Fatalf("NewDownloader failed: %v", err)
}

err := d.getFileSize()
err = d.getFileSize()
if err != nil {
t.Fatalf("getFileSize() error = %v, want nil", err)
}
Expand All @@ -36,10 +39,13 @@ func TestGetFileSizeRetry(t *testing.T) {
// Fail first 2 attempts, succeed on 3rd
mock.SetMaxFailures(2)

d := NewDownloader(mock.URL(), 4, 1024, "", 0, false, 5, 50*time.Millisecond, 1024*1024*1024)
d, err := NewDownloader(mock.URL(), 4, 1024, "", 0, false, 5, 50*time.Millisecond, 1024*1024*1024)
if err != nil {
t.Fatalf("NewDownloader failed: %v", err)
}

start := time.Now()
err := d.getFileSize()
err = d.getFileSize()
elapsed := time.Since(start)

if err != nil {
Expand All @@ -66,9 +72,12 @@ func TestGetFileSizeExhaustedRetries(t *testing.T) {
// Fail more times than max retries
mock.SetMaxFailures(10)

d := NewDownloader(mock.URL(), 4, 1024, "", 0, false, 3, 50*time.Millisecond, 1024*1024*1024)
d, err := NewDownloader(mock.URL(), 4, 1024, "", 0, false, 3, 50*time.Millisecond, 1024*1024*1024)
if err != nil {
t.Fatalf("NewDownloader failed: %v", err)
}

err := d.getFileSize()
err = d.getFileSize()
if err == nil {
t.Fatal("getFileSize() error = nil, want error when retries exhausted")
}
Expand All @@ -87,13 +96,16 @@ func TestDownloadChunk(t *testing.T) {
mock := NewMockHTTPServer(content)
defer mock.Close()

d := NewDownloader(mock.URL(), 4, 1024, "", 0, false, 3, 100*time.Millisecond, 1024*1024*1024)
d, err := NewDownloader(mock.URL(), 4, 1024, "", 0, false, 3, 100*time.Millisecond, 1024*1024*1024)
if err != nil {
t.Fatalf("NewDownloader failed: %v", err)
}

ctx := context.Background()
var buf bytes.Buffer

// Download first 1KB
err := d.downloadChunk(ctx, 0, 1023, &buf)
err = d.downloadChunk(ctx, 0, 1023, &buf)
if err != nil {
t.Fatalf("downloadChunk() error = %v, want nil", err)
}
Expand All @@ -112,13 +124,16 @@ func TestDownloadChunkMiddle(t *testing.T) {
mock := NewMockHTTPServer(content)
defer mock.Close()

d := NewDownloader(mock.URL(), 4, 1024, "", 0, false, 3, 100*time.Millisecond, 1024*1024*1024)
d, err := NewDownloader(mock.URL(), 4, 1024, "", 0, false, 3, 100*time.Millisecond, 1024*1024*1024)
if err != nil {
t.Fatalf("NewDownloader failed: %v", err)
}

ctx := context.Background()
var buf bytes.Buffer

// Download bytes 5000-5999
err := d.downloadChunk(ctx, 5000, 5999, &buf)
err = d.downloadChunk(ctx, 5000, 5999, &buf)
if err != nil {
t.Fatalf("downloadChunk() error = %v, want nil", err)
}
Expand All @@ -139,12 +154,15 @@ func TestDownloadChunkRetry(t *testing.T) {
// Fail first 2 attempts
mock.SetMaxFailures(2)

d := NewDownloader(mock.URL(), 4, 1024, "", 0, false, 5, 50*time.Millisecond, 1024*1024*1024)
d, err := NewDownloader(mock.URL(), 4, 1024, "", 0, false, 5, 50*time.Millisecond, 1024*1024*1024)
if err != nil {
t.Fatalf("NewDownloader failed: %v", err)
}

ctx := context.Background()
var buf bytes.Buffer

err := d.downloadChunk(ctx, 0, 1023, &buf)
err = d.downloadChunk(ctx, 0, 1023, &buf)
if err != nil {
t.Fatalf("downloadChunk() with retries error = %v, want nil", err)
}
Expand All @@ -169,13 +187,16 @@ func TestDownloadChunkTimeout(t *testing.T) {
// Add delay longer than timeout
mock.SetRequestDelay(6 * time.Minute)

d := NewDownloader(mock.URL(), 4, 1024, "", 0, false, 1, 100*time.Millisecond, 1024*1024*1024)
d, err := NewDownloader(mock.URL(), 4, 1024, "", 0, false, 1, 100*time.Millisecond, 1024*1024*1024)
if err != nil {
t.Fatalf("NewDownloader failed: %v", err)
}

ctx := context.Background()
var buf bytes.Buffer

start := time.Now()
err := d.downloadChunk(ctx, 0, 1023, &buf)
err = d.downloadChunk(ctx, 0, 1023, &buf)
elapsed := time.Since(start)

if err == nil {
Expand All @@ -197,15 +218,18 @@ func TestDownloadChunkCancellation(t *testing.T) {
// Add delay to ensure we can cancel
mock.SetRequestDelay(100 * time.Millisecond)

d := NewDownloader(mock.URL(), 4, 1024, "", 0, false, 3, 50*time.Millisecond, 1024*1024*1024)
d, err := NewDownloader(mock.URL(), 4, 1024, "", 0, false, 3, 50*time.Millisecond, 1024*1024*1024)
if err != nil {
t.Fatalf("NewDownloader failed: %v", err)
}

ctx, cancel := context.WithCancel(context.Background())
var buf bytes.Buffer

// Cancel immediately
cancel()

err := d.downloadChunk(ctx, 0, 1023, &buf)
err = d.downloadChunk(ctx, 0, 1023, &buf)
if err == nil {
t.Fatal("downloadChunk() error = nil, want cancellation error")
}
Expand All @@ -217,13 +241,16 @@ func TestDownloadChunkBoundary(t *testing.T) {
mock := NewMockHTTPServer(content)
defer mock.Close()

d := NewDownloader(mock.URL(), 4, 512, "", 0, false, 3, 100*time.Millisecond, 1024*1024*1024)
d, err := NewDownloader(mock.URL(), 4, 512, "", 0, false, 3, 100*time.Millisecond, 1024*1024*1024)
if err != nil {
t.Fatalf("NewDownloader failed: %v", err)
}

ctx := context.Background()
var buf bytes.Buffer

// Download last chunk (bytes 512-1023)
err := d.downloadChunk(ctx, 512, 1023, &buf)
err = d.downloadChunk(ctx, 512, 1023, &buf)
if err != nil {
t.Fatalf("downloadChunk() boundary error = %v, want nil", err)
}
Expand All @@ -244,15 +271,18 @@ func TestDownloadChunkServerNoRangeSupport(t *testing.T) {
// Disable range support
mock.SetSupportsRanges(false)

d := NewDownloader(mock.URL(), 4, 1024, "", 0, false, 3, 100*time.Millisecond, 1024*1024*1024)
d, err := NewDownloader(mock.URL(), 4, 1024, "", 0, false, 3, 100*time.Millisecond, 1024*1024*1024)
if err != nil {
t.Fatalf("NewDownloader failed: %v", err)
}

ctx := context.Background()
var buf bytes.Buffer

// Try to download a chunk - should fail because server doesn't support ranges
// When server doesn't support ranges, it returns 200 OK with full content,
// which is incompatible with parallel downloads and would corrupt the file
err := d.downloadChunk(ctx, 1024, 2047, &buf)
err = d.downloadChunk(ctx, 1024, 2047, &buf)
if err == nil {
t.Fatal("downloadChunk() error = nil, want error for server without range support")
}
Expand All @@ -266,10 +296,13 @@ func TestDownloadChunkServerNoRangeSupport(t *testing.T) {

// TestRetryWithBackoffSuccess tests retry logic succeeds immediately
func TestRetryWithBackoffSuccess(t *testing.T) {
d := NewDownloader("http://example.com/test", 4, 1024, "", 0, false, 3, 100*time.Millisecond, 1024*1024*1024)
d, err := NewDownloader("http://example.com/test", 4, 1024, "", 0, false, 3, 100*time.Millisecond, 1024*1024*1024)

if err != nil {
t.Fatalf("NewDownloader failed: %v", err)
}
attempts := 0
err := d.retryWithBackoff(context.Background(), "test operation", func() error {
err = d.retryWithBackoff(context.Background(), "test operation", func() error {
attempts++
return nil // Success on first try
})
Expand All @@ -285,10 +318,13 @@ func TestRetryWithBackoffSuccess(t *testing.T) {

// TestRetryWithBackoffEventualSuccess tests retry succeeds after failures
func TestRetryWithBackoffEventualSuccess(t *testing.T) {
d := NewDownloader("http://example.com/test", 4, 1024, "", 0, false, 5, 50*time.Millisecond, 1024*1024*1024)
d, err := NewDownloader("http://example.com/test", 4, 1024, "", 0, false, 5, 50*time.Millisecond, 1024*1024*1024)

if err != nil {
t.Fatalf("NewDownloader failed: %v", err)
}
attempts := 0
err := d.retryWithBackoff(context.Background(), "test operation", func() error {
err = d.retryWithBackoff(context.Background(), "test operation", func() error {
attempts++
if attempts < 3 {
return errors.New("temporary failure")
Expand All @@ -307,11 +343,14 @@ func TestRetryWithBackoffEventualSuccess(t *testing.T) {

// TestRetryWithBackoffAllFail tests behavior when all retries fail
func TestRetryWithBackoffAllFail(t *testing.T) {
d := NewDownloader("http://example.com/test", 4, 1024, "", 0, false, 3, 50*time.Millisecond, 1024*1024*1024)
d, err := NewDownloader("http://example.com/test", 4, 1024, "", 0, false, 3, 50*time.Millisecond, 1024*1024*1024)

if err != nil {
t.Fatalf("NewDownloader failed: %v", err)
}
attempts := 0
testErr := errors.New("persistent failure")
err := d.retryWithBackoff(context.Background(), "test operation", func() error {
err = d.retryWithBackoff(context.Background(), "test operation", func() error {
attempts++
return testErr
})
Expand All @@ -329,8 +368,11 @@ func TestRetryWithBackoffAllFail(t *testing.T) {

// TestRetryWithBackoffCancellation tests context cancellation during retry
func TestRetryWithBackoffCancellation(t *testing.T) {
d := NewDownloader("http://example.com/test", 4, 1024, "", 0, false, 10, 100*time.Millisecond, 1024*1024*1024)
d, err := NewDownloader("http://example.com/test", 4, 1024, "", 0, false, 10, 100*time.Millisecond, 1024*1024*1024)

if err != nil {
t.Fatalf("NewDownloader failed: %v", err)
}
ctx, cancel := context.WithCancel(context.Background())

attempts := 0
Expand All @@ -348,7 +390,7 @@ func TestRetryWithBackoffCancellation(t *testing.T) {
time.Sleep(150 * time.Millisecond)
cancel()

err := <-errChan
err = <-errChan
if err == nil {
t.Fatal("retryWithBackoff() error = nil, want cancellation error")
}
Expand All @@ -370,14 +412,17 @@ func TestRateLimitedReader(t *testing.T) {

// Set bandwidth limit to 10KB/s
bandwidthLimit := int64(10 * 1024)
d := NewDownloader(mock.URL(), 4, 1024, "", bandwidthLimit, false, 3, 100*time.Millisecond, 1024*1024*1024)
d, err := NewDownloader(mock.URL(), 4, 1024, "", bandwidthLimit, false, 3, 100*time.Millisecond, 1024*1024*1024)
if err != nil {
t.Fatalf("NewDownloader failed: %v", err)
}

ctx := context.Background()
var buf bytes.Buffer

start := time.Now()
// Download 30KB (should take ~3 seconds at 10KB/s after burst)
err := d.downloadChunk(ctx, 0, 30*1024-1, &buf)
err = d.downloadChunk(ctx, 0, 30*1024-1, &buf)
elapsed := time.Since(start)

if err != nil {
Expand Down
Loading