From efa5e8fcc1abe17a61629708185fd260b2a56433 Mon Sep 17 00:00:00 2001
From: Vijay Misal <misalvijay153@gmail.com>
Date: Sun, 16 Aug 2026 15:09:04 +0530
Subject: [PATCH] vfscache: fix log message growing without bound on repeated
 write errors

Write() overwrote a successful write's nil error with the stale
lastErr returned by kickWaiters() once the downloader had recorded
too many errors. download() then wrapped that stale error again and
stored it back as the new lastErr, so every subsequent write added
another "vfs reader: failed to write to cache file:" prefix - fixes #4998
---
 vfs/vfscache/downloaders/downloaders.go      |  9 +++--
 vfs/vfscache/downloaders/downloaders_test.go | 41 ++++++++++++++++++++
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/vfs/vfscache/downloaders/downloaders.go b/vfs/vfscache/downloaders/downloaders.go
index 39cd4e883..6e37adfcb 100644
--- a/vfs/vfscache/downloaders/downloaders.go
+++ b/vfs/vfscache/downloaders/downloaders.go
@@ -457,15 +457,18 @@ func (dl *downloader) Write(p []byte) (n int, err error) {
 	// defer log.Trace(dl.dls.src, "p_len=%d", len(p))("n=%d, err=%v", &n, &err)
 
 	// Kick the waiters on exit if some characters received
+	//
+	// Only logs kickWaiters failures rather than returning them as this
+	// Write's own error: kickWaiters can return the downloaders' stale
+	// accumulated lastErr, which would otherwise fail this write (even
+	// when it succeeded) and get wrapped again by download(), growing
+	// unboundedly on every subsequent call while lastErr stays set.
 	defer func() {
 		if n <= 0 {
 			return
 		}
 		if waitErr := dl.dls.kickWaiters(); waitErr != nil {
 			fs.Errorf(dl.dls.src, "vfs cache: download write: failed to kick waiters: %v", waitErr)
-			if err == nil {
-				err = waitErr
-			}
 		}
 	}()
 
diff --git a/vfs/vfscache/downloaders/downloaders_test.go b/vfs/vfscache/downloaders/downloaders_test.go
index 592c3f470..7ad626927 100644
--- a/vfs/vfscache/downloaders/downloaders_test.go
+++ b/vfs/vfscache/downloaders/downloaders_test.go
@@ -2,6 +2,8 @@ package downloaders
 
 import (
 	"context"
+	"errors"
+	"fmt"
 	"io"
 	"sync"
 	"testing"
@@ -151,4 +153,43 @@ func TestDownloaders(t *testing.T) {
 			t.Fatal("Download did not return: the waiter was never dispatched")
 		}
 	})
+
+	// A successful write must not be turned into a failure by a stale
+	// error left over from a previous problem (e.g. the cache running
+	// out of space earlier). Regression test: this used to make Write
+	// return dls.lastErr even when the write itself succeeded, which
+	// then got wrapped again by download() and stored back as the new
+	// lastErr - causing the wrapped error message to grow without bound
+	// on every subsequent write.
+	t.Run("WriteDoesNotFailOnStaleError", func(t *testing.T) {
+		item, dls := newTest()
+		defer cancel(dls)
+
+		dls.mu.Lock()
+		dls.errorCount = maxErrorCount + 1
+		dls.lastErr = fmt.Errorf("vfs reader: failed to write to cache file: %w", errors.New("no space left on device"))
+		// A waiter for a range that isn't downloaded yet, so kickWaiters
+		// doesn't dispatch it immediately and reaches the errorCount check.
+		dls.waiters = append(dls.waiters, waiter{
+			r:       ranges.Range{Pos: 10 * 1024 * 1024, Size: 250},
+			errChan: make(chan error, 1),
+		})
+		dls.mu.Unlock()
+
+		dl := &downloader{
+			dls:       dls,
+			quit:      make(chan struct{}),
+			kick:      make(chan struct{}, 1),
+			offset:    0,
+			maxOffset: 1024,
+		}
+
+		p := make([]byte, 16)
+		_, err := io.ReadFull(readers.NewPatternReader(item.size), p)
+		require.NoError(t, err)
+
+		n, err := dl.Write(p)
+		require.NoError(t, err, "a successful write must not fail due to unrelated stale state")
+		assert.Equal(t, len(p), n)
+	})
 }