fix: await function template downloads in functions-create - #8465
Conversation
`downloadFromURL` wrote each template file with `res.body?.pipe(dest)`, which was never awaited. `Promise.all` therefore resolved before any write had finished, and the `npm i` that follows ran against a possibly incomplete function directory. `await pipeline(...)` closes the race. A non-OK response was also piped straight to disk, silently writing an error page where the template should be. That is now an explicit throw. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
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)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: Your plan provides up to 4 included reviews per hour; 1 remains after this review. 📝 SummarySummary by CodeRabbit
WalkthroughThe template download flow now validates HTTP responses and requires a response body. It throws an HTTP error for failed or empty responses. It awaits Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to URL-based function template downloads now reject invalid responses and wait for file writes to finish, preventing incomplete or error-content templates. The change is ready to merge. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
commit: |
Two bugs in
downloadFromURL(src/commands/functions/functions-create.ts), both on thenetlify functions:create --urlpath.Template downloads were never awaited. Files were written with
res.body?.pipe(dest)— noawait, soPromise.allresolved before any write had finished. Thecp.exec('npm i', ...)that immediately follows could therefore run against a partially-written function directory.await pipeline(res.body, dest)closes the race.A failed download was written to disk as the template. There was no
res.okcheck, so a 404 or 500 from GitHub had its error-page body piped into the function file and the command reported success. That's now an explicit throw, which the surroundingcatchturns into the existingError while retrieving ${downloadUrl}message.The
node-fetchimport is unchanged — node-fetch v3'sres.bodyis a NodeReadabletypedNodeJS.ReadableStream | null, which satisfiesPipelineSource, so this is purely the bug fix.Verification
npm run typecheck,npm run lint,npm run format:check— cleannpm run test:unit— 499/500, matching the baseline on unmodifiedmainexactly (thegenerate-autocompletionsnapshot failure is pre-existing; I reproduced it onmainat97fd77d3c)No
package-lock.jsonchange.Split out of #8453, which bundled this with unrelated work.
🤖 Generated with Claude Code