Refresh buffered files when source changes#111
Conversation
Reviewer's GuideAligns buffered file caching with streamed file freshness checks by always stat-ing the source file, refreshing metadata and buffer (including MD5/ETag) when mtime or size changes, and clearing any cached gzip buffer; adds a regression test to verify buffered responses update when the underlying file changes. Sequence diagram for refreshed buffered file caching on source changesequenceDiagram
actor Client
participant KoaApp
participant staticCacheMiddleware
participant fs
participant crypto
Client->>KoaApp: HTTP GET /asset
KoaApp->>staticCacheMiddleware: handle request
staticCacheMiddleware->>fs: stat(file.path)
fs-->>staticCacheMiddleware: stats
alt [stats.mtime or stats.size changed]
staticCacheMiddleware->>staticCacheMiddleware: file.mtime = stats.mtime
staticCacheMiddleware->>staticCacheMiddleware: file.length = stats.size
staticCacheMiddleware->>staticCacheMiddleware: file.md5 = null
staticCacheMiddleware->>staticCacheMiddleware: file.zipBuffer = null
alt [file.buffer exists]
staticCacheMiddleware->>fs: readFile(file.path)
fs-->>staticCacheMiddleware: file.buffer
staticCacheMiddleware->>crypto: createHash(md5).update(file.buffer).digest(base64)
crypto-->>staticCacheMiddleware: md5
staticCacheMiddleware->>staticCacheMiddleware: file.md5 = md5
end
end
staticCacheMiddleware-->>Client: HTTP response (buffered or streamed)
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
More reviews will be available in 24 minutes and 7 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe per-request freshness check inside 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The freshness logic for updating
file.mtime,file.length,file.md5, andfile.zipBufferis now embedded directly in the middleware; consider extracting this into a shared helper so it can’t drift from the streamed-file path and is easier to test in isolation. - In the new
should refresh buffered files when the source changestest, the file cleanup relies on multiplefs.unlinkSynccalls within nested callbacks; using atry/finallyor MochaafterEachhook to guarantee cleanup would make the test more robust against early failures.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The freshness logic for updating `file.mtime`, `file.length`, `file.md5`, and `file.zipBuffer` is now embedded directly in the middleware; consider extracting this into a shared helper so it can’t drift from the streamed-file path and is easier to test in isolation.
- In the new `should refresh buffered files when the source changes` test, the file cleanup relies on multiple `fs.unlinkSync` calls within nested callbacks; using a `try/finally` or Mocha `afterEach` hook to guarantee cleanup would make the test more robust against early failures.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/index.js (1)
153-190: ⚡ Quick winAdd a regression assertion for gzip-buffer invalidation on refresh.
This test validates buffer + ETag refresh, but it doesn’t cover the
zipBufferinvalidation path described in the PR objective. Consider extending this case with a gzip request before/after mutation to ensure stale compressed bytes are not reused.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/index.js` around lines 153 - 190, The test case for buffer freshness does not verify that the gzip-compressed buffer (zipBuffer) is properly invalidated when the source file changes. Extend the test by adding gzip encoding requests both before and after the file mutation to ensure that stale compressed bytes are not reused. Specifically, make an additional request with Accept-Encoding: gzip header before the fs.writeFileSync call that mutates the file, capture the gzip buffer from the files object, then after the file mutation and the second request, verify that the new gzip buffer is different from the stale one to confirm the compressed cache was properly invalidated and refreshed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@index.js`:
- Around line 84-93: The fs.stat and fs.readFile calls in the freshness check
block can throw errors when files are deleted or rotated between requests,
currently causing unhandled 500 errors. Wrap the entire stats checking and file
reading logic (starting from the fs.stat(file.path) call through the
crypto.createHash operations) in a try-catch block that specifically handles
ENOENT errors by treating them as cache invalidation and calling next() to allow
the request to proceed (resulting in a 404 downstream), while rethrowing any
other unexpected I/O errors to maintain proper error handling for genuine
issues.
---
Nitpick comments:
In `@test/index.js`:
- Around line 153-190: The test case for buffer freshness does not verify that
the gzip-compressed buffer (zipBuffer) is properly invalidated when the source
file changes. Extend the test by adding gzip encoding requests both before and
after the file mutation to ensure that stale compressed bytes are not reused.
Specifically, make an additional request with Accept-Encoding: gzip header
before the fs.writeFileSync call that mutates the file, capture the gzip buffer
from the files object, then after the file mutation and the second request,
verify that the new gzip buffer is different from the stale one to confirm the
compressed cache was properly invalidated and refreshed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 514d73c6-e12d-40f9-833c-e43a8ccd9423
📒 Files selected for processing (2)
index.jstest/index.js
1 participant
Closes #96.
Buffered files now use the same freshness check as streamed files. When the source file mtime or size changes, the cached metadata is refreshed; buffered responses also reload the file contents and recalculate the MD5/ETag before serving.
This also clears any cached gzip buffer when the source changes so compressed responses are regenerated from the current file contents.
Verification:
./node_modules/.bin/mocha --require should --require should-http --harmony --grep "refresh buffered files"npm testgit diff --checkSummary by Sourcery
Refresh cached static file metadata and contents when the source file changes, ensuring buffered responses stay in sync with the underlying files.
New Features:
Bug Fixes:
Tests:
Summary by CodeRabbit
Bug Fixes
Tests