Skip to content

fix: Ensure response body is always closed in async revalidation. #47

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
sendya merged 1 commit into from
Mar 11, 2026
Merged

fix: Ensure response body is always closed in async revalidation. #47

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
43 changes: 22 additions & 21 deletions server/middleware/caching/caching_revalidate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ func calculateSoftTTL(respUnix, expiresAt int64, fuzzyRate float64) int64 {
if hardTTL <= 0 {
return expiresAt
}

// Ensure fuzzy rate is in valid range (0, 1.0]
if fuzzyRate <= 0 || fuzzyRate > 1 {
fuzzyRate = 0.8 // default to 0.8 if invalid
}

softTTL := int64(float64(hardTTL) * fuzzyRate)
return respUnix + softTTL
}
Expand All @@ -48,22 +48,22 @@ func shouldTriggerFuzzyRefresh(now, softTTL, hardTTL int64) bool {
// Before soft TTL, no refresh needed
return false
}

if now >= hardTTL {
// After hard TTL, force refresh (handled by hasExpired)
return false
}

// In the fuzzy refresh zone [soft_ttl, hard_ttl)
// Calculate linear probability: P = (now - soft_ttl) / (hard_ttl - soft_ttl)
totalWindow := float64(hardTTL - softTTL)
if totalWindow <= 0 {
return false
}

elapsed := float64(now - softTTL)
probability := elapsed / totalWindow

// Random trigger based on probability using math/rand/v2 which is thread-safe
return rand.Float64() < probability
}
Expand All @@ -72,14 +72,14 @@ func (r *RevalidateProcessor) Lookup(c *Caching, req *http.Request) (bool, error
if c.md == nil {
return false, nil
}

now := time.Now().Unix()
hardTTL := c.md.ExpiresAt

// Fuzzy Refresh Logic
if c.opt.FuzzyRefresh && c.opt.FuzzyRefreshRate > 0 {
softTTL := calculateSoftTTL(c.md.RespUnix, c.md.ExpiresAt, c.opt.FuzzyRefreshRate)

// Check if we're in the fuzzy refresh zone [soft_ttl, hard_ttl)
if now >= softTTL && now < hardTTL {
// We're in the fuzzy refresh zone
Expand All @@ -90,17 +90,17 @@ func (r *RevalidateProcessor) Lookup(c *Caching, req *http.Request) (bool, error
c.id.Key(),
time.Unix(softTTL, 0).Format(time.DateTime),
time.Unix(hardTTL, 0).Format(time.DateTime))

// Trigger async revalidation in background
go r.asyncRevalidate(c, req)
}
}

// Still return cache hit - serve stale content while refreshing
return true, nil
}
}

// check if metadata is expired (hard expiration).
if !hasExpired(c.md) {
return true, nil
Expand Down Expand Up @@ -236,38 +236,39 @@ func (r *RevalidateProcessor) asyncRevalidate(c *Caching, req *http.Request) {
// Create a background context with timeout
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()

// Clone the request for background processing
bgReq := req.Clone(ctx)

// Set conditional headers for revalidation
if c.md.Headers.Get("ETag") != "" {
bgReq.Header.Set("If-None-Match", c.md.Headers.Get("ETag"))
}
if c.md.Headers.Get("Last-Modified") != "" {
bgReq.Header.Set("If-Modified-Since", c.md.Headers.Get("Last-Modified"))
}

// Remove Range header for full object revalidation
bgReq.Header.Del("Range")

c.log.Debugf("async fuzzy refresh started for object: %s", c.id.Key())

// Perform the upstream request
resp, err := c.doProxy(bgReq, false)
defer closeBody(resp) // always check resp nil
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

The inline comment on the new defer closeBody(resp) is misleading: closeBody already guards nil responses, and the real reason to defer before the error check is that doProxy can return a non-nil resp alongside a non-nil err (so the body must still be closed). Consider updating/removing the comment to reflect that behavior to avoid confusion for future readers.

Suggested change
defer closeBody(resp) // always check resp nil
// Defer closing the response body even if doProxy returns a non-nil resp alongside a non-nil err.
defer closeBody(resp)

Copilot uses AI. Check for mistakes.

if err != nil {
c.log.Warnf("async fuzzy refresh failed for object %s: %v", c.id.Key(), err)
return
}
defer closeBody(resp)


// Handle 304 Not Modified - just update freshness metadata
if resp.StatusCode == http.StatusNotModified {
r.freshness(c, resp)
c.log.Debugf("async fuzzy refresh completed (304) for object: %s", c.id.Key())
return
}

// For non-304 responses, the content has changed
// The doProxy method has already wrapped the response body with cache writing logic
// We need to consume the body to trigger the cache update
Expand All @@ -282,7 +283,7 @@ func (r *RevalidateProcessor) asyncRevalidate(c *Caching, req *http.Request) {
c.log.Debugf("async fuzzy refresh completed (%d) for object: %s - content updated", resp.StatusCode, c.id.Key())
return
}

c.log.Debugf("async fuzzy refresh completed (%d) for object: %s", resp.StatusCode, c.id.Key())
}

Expand Down