Repository navigation
fix(static): respond with 404 when a public asset is missing on disk - #4588
Conversation
|
@danielroe is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe static asset reader now returns ChangesStatic asset handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to Missing static assets are handled as 404 responses, with no concrete unresolved merge-readiness risk identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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 |
commit: |
Translate ENOENT inside the generated node reader instead of the runtime handler, so all three readers share one contract: resolving with no value means the manifest entry has no data behind it. This keeps Node errno checks out of `src/runtime`, restores the handler to a synchronous fallthrough for non-asset requests, and covers the `inline` reader, which already returned no data for a missing payload and until now served it as an empty 200.
Both reported cases (nuxt#29078, nuxt#33033) prune the public dir on purpose - uploading it to a CDN, or stripping sourcemaps after sending them to Sentry - so the missing file is a standing configuration rather than a one-off fault, and warning per read only spams the log.
|
thank you @pi0 - this is a better approach ❤️ |
🔗 Linked issue
resolves nuxt/nuxt#33033
resolves nuxt/nuxt#29078
❓ Type of change
📚 Description
when a file can't be read because it can't be read from disk, we can return a 404 instead of a 500 error, which seems wrong.
alternatively, we could always return a 404 when the asset can't be read, if you didn't want to tie this too closely to fs semantics.
... or you may have another better idea!
📝 Checklist