⚡ Bolt: [performance improvement] Refactor preview server to use asynchronous I/O#89
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughPreview HTTP server refactored from synchronous to asynchronous file operations. A new ChangesElectron Preview Server Async File Operations
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 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. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
There was a problem hiding this comment.
Pull request overview
Refactors the Electron main-process preview server to avoid blocking synchronous filesystem calls while serving static assets, improving responsiveness and concurrency when loading preview content.
Changes:
- Replaced
existsSync/statSyncchecks with async equivalents (fs.promises.access/fs.promises.stat). - Switched from
readFileSyncresponses to streaming files viafs.createReadStream().pipe(res)to reduce memory overhead. - Documented the “avoid sync fs in Electron main process” performance learning in
.jules/bolt.md.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/desktop/src/main/preview-server.ts | Converts preview server filesystem operations to async + stream-based responses. |
| .jules/bolt.md | Adds a Bolt learning entry about avoiding sync fs calls in Electron main process code paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private async handleRequest(req: any, res: any): Promise<void> { | ||
| const urlPath = req.url.split('?')[0]; | ||
| let filePath = path.join(this.rootDir, urlPath === '/' ? 'index.html' : urlPath); | ||
|
|
||
| if (!fs.existsSync(filePath)) { | ||
| // ⚡ Bolt: Replace synchronous file operations with async promises to prevent blocking the main thread |
| private async handleRequest(req: any, res: any): Promise<void> { | ||
| const urlPath = req.url.split('?')[0]; | ||
| let filePath = path.join(this.rootDir, urlPath === '/' ? 'index.html' : urlPath); | ||
|
|
| 'Content-Type': contentType, | ||
| 'Cache-Control': 'no-cache', | ||
| }); | ||
| res.end(content); | ||
| } catch { | ||
| res.writeHead(500); | ||
| res.end('Failed to read file'); | ||
| } | ||
| }); | ||
| readStream.pipe(res); |
| @@ -61,3 +61,6 @@ | |||
| ## 2026-05-19 - Top-Level Interval Anti-Pattern (SpinnerVerb) | |||
| **Learning:** Storing a slow interval state like `spinnerVerb` (every 2.4s) at the top-level `App` component still forces a full application tree O(N) cascade re-render every tick. Even if it is not as fast as a 250ms timer, it causes noticeable micro-stuttering during UI interactions and when large lists are rendered because it triggers diffs across the entire application including Heavy `ChatPanel` and `TabbedRightPanel`. | |||
| **Action:** Always extract even slow interval-driven decorative states into isolated leaf components (like `SpinnerVerbDisplay`) wrapped in `React.memo()`. Removing the prop-drilling entirely isolates the re-renders specifically to the UI span rendering the changing string. | |||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@packages/desktop/src/main/preview-server.ts`:
- Line 82: The async method handleRequest can reject and its caller currently
invokes it without awaiting or catching the returned Promise; update either the
caller or handleRequest so rejections are always handled: either (A) change the
caller that calls handleRequest to await it or call handleRequest(...).catch(err
=> { send an error response and log the error }) or (B) add a top-level
try/catch inside handleRequest to catch any errors and send an appropriate HTTP
response (and log) before returning; reference the handleRequest function and
the code path where it is invoked so you handle all promise rejections and
ensure the response is always sent.
- Around line 121-126: The readStream error handler currently always calls
res.end('Failed to read file'), which corrupts binary responses when headers
have already been sent; update the readStream.on('error') handler (the listener
attached to readStream) to check res.headersSent first: if headers have not been
sent, write a 500 status and call res.end with the error message, otherwise call
res.destroy() (or res.end() without writing additional body) to terminate the
connection; ensure the code references the existing readStream.on('error')
listener and uses res.headersSent, res.writeHead, and res.destroy to avoid
appending text to an in-progress binary response.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 788f4305-5ac9-4711-9e5f-c0b9939225df
📒 Files selected for processing (2)
.jules/bolt.mdpackages/desktop/src/main/preview-server.ts
| } | ||
|
|
||
| private handleRequest(req: any, res: any): void { | ||
| private async handleRequest(req: any, res: any): Promise<void> { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/desktop/src/main/preview-server.ts | head -150Repository: iotserver24/Xibecode
Length of output: 4504
Handle rejected promises from the async request handler.
Line 82 makes handleRequest asynchronous, but line 45 invokes it without awaiting or catching the returned promise. Any uncaught error in handleRequest becomes an unhandled promise rejection, leaving requests without a proper response.
Suggested fix
- this.server = createServer((req, res) => {
- this.handleRequest(req, res);
- });
+ this.server = createServer((req, res) => {
+ void this.handleRequest(req, res).catch(() => {
+ if (!res.headersSent) {
+ res.writeHead(500);
+ }
+ res.end('Internal server error');
+ });
+ });🤖 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 `@packages/desktop/src/main/preview-server.ts` at line 82, The async method
handleRequest can reject and its caller currently invokes it without awaiting or
catching the returned Promise; update either the caller or handleRequest so
rejections are always handled: either (A) change the caller that calls
handleRequest to await it or call handleRequest(...).catch(err => { send an
error response and log the error }) or (B) add a top-level try/catch inside
handleRequest to catch any errors and send an appropriate HTTP response (and
log) before returning; reference the handleRequest function and the code path
where it is invoked so you handle all promise rejections and ensure the response
is always sent.
| readStream.on('error', () => { | ||
| if (!res.headersSent) { | ||
| res.writeHead(500); | ||
| } | ||
| res.end('Failed to read file'); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "preview-server.ts" -type fRepository: iotserver24/Xibecode
Length of output: 110
🏁 Script executed:
sed -n '115,135p' ./packages/desktop/src/main/preview-server.tsRepository: iotserver24/Xibecode
Length of output: 681
🌐 Web query:
Node.js res.end() after headersSent true behavior
💡 Result:
In Node.js, calling res.end when headersSent is true is a standard and safe operation [1][2]. The res.end method is the required finalizer for an HTTP response; it signals to the server that all headers and body data have been sent and that the response is complete [3][4]. When res.end is called: - If headers have not yet been sent (headersSent is false), Node.js will automatically send them before finalizing the response [1][5]. - If headers have already been sent (headersSent is true), res.end simply completes the response stream [1][3]. The common error ERR_HTTP_HEADERS_SENT occurs not because of calling res.end when headers are already sent, but because of attempting to modify headers (e.g., using res.setHeader or res.writeHead) or sending additional response data after the response has already been finalized [6][7][2]. Best practices for handling this state include: - Checking res.headersSent before attempting to set headers or send error responses in asynchronous code paths [2][8]. - Ensuring that each request handler has exactly one response path to avoid attempting to send multiple responses [7][2]. - Using return statements after calling res.end or other response-sending methods to prevent subsequent code from executing and attempting to modify the already-sent response [1][2][9].
Citations:
- 1: https://thelinuxcode.com/express-resend-in-2026-ending-http-responses-precisely-without-surprises/
- 2: https://oneuptime.com/blog/post/2026-01-25-fix-err-http-headers-sent/view
- 3: https://nodejs.org/api/http.html
- 4: https://www.w3schools.com/nodejs/ref_response.asp
- 5: https://stackoverflow.com/questions/28289905/how-does-expresss-res-end-work
- 6: https://runebook.dev/en/articles/node/errors/err_http_headers_sent
- 7: https://sentry.io/answers/error-err-http-headers-sent-cannot-set-headers-after-they-are-sent-to-the-client/
- 8: https://runebook.dev/en/articles/node/http/outgoingmessageheaderssent
- 9: https://endyourif.com/cant-set-headers-after-they-are-sent
🌐 Web query:
Node.js stream error handler res.destroy vs res.end when piping
💡 Result:
When piping streams in Node.js, the choice between res.destroy and res.end depends on the context of the error and the state of the response [1][2]. The.pipe method does not automatically propagate errors or clean up streams [3][4]. If an error occurs in a piped stream, the destination (such as an HTTP response) is often left hanging, keeping the socket open and the client waiting [1][5]. res.destroy(err) Use this when a fatal error occurs that prevents the response from completing successfully [6][5]. Calling.destroy immediately closes the underlying socket, which is appropriate if you cannot send a valid response body or if the stream is in an invalid state [6][5]. It signals that the connection should be terminated abruptly [5]. res.end Use this when you need to gracefully terminate the response, typically after you have already sent some data or when you want to send a final error message (e.g., a 500 status code) before closing [1][2]. If headers have not been sent, you can set an error status code and then call.end to inform the client of the failure [5]. Best Practice: Use stream.pipeline The modern, recommended approach is to use stream.pipeline instead of.pipe [3][7][4]. The pipeline utility automatically handles error propagation and cleanup [4]. When an error occurs, it automatically calls.destroy on all streams in the pipeline, preventing resource leaks and hanging sockets [4]. Example using pipeline: const { pipeline } = require('node:stream'); pipeline(sourceStream, res, (err) => { if (err) { console.error('Pipeline failed:', err); // If headers haven't been sent, you can send a 500 error if (!res.headersSent) { res.statusCode = 500; res.end('Internal Server Error'); } else { // Otherwise, destroy the response to close the connection res.destroy(err); } } }); In summary, prefer pipeline to avoid manual error handling [3][8]. If you must use.pipe, you are responsible for manually attaching error listeners to all streams and deciding whether to call.end (for a graceful, potentially error-informed close) or.destroy (for an immediate, forced termination) based on whether headers have already been sent [1][3][5].
Citations:
- 1: https://stackoverflow.com/questions/68331843/in-express-js-how-do-you-properly-end-a-stream-response-that-throws-an-error
- 2: https://thelinuxcode.com/express-resend-in-2026-ending-http-responses-precisely-without-surprises/
- 3: https://fixdevs.com/blog/node-stream-error/
- 4: https://www.thenodebook.com/streams/modern-pipelines-error-handling
- 5: https://medium.com/@bhagyarana80/node-streams-and-the-11-errors-that-keep-sockets-hanging-80dcadb82eef
- 6: stream.pipeline destroys writable stream when error is occurred nodejs/node#26311
- 7: https://dev.to/armorbreak/nodejs-streams-the-practical-guide-kia
- 8: https://dev.to/ruben_alapont/error-handling-in-nodejs-streams-best-practices-dhb
🏁 Script executed:
head -n 130 ./packages/desktop/src/main/preview-server.ts | tail -n 20 | cat -nRepository: iotserver24/Xibecode
Length of output: 857
Move the error response body inside the headersSent check.
Line 125 unconditionally calls res.end('Failed to read file') even when headersSent is true. If the stream errors after piping begins (after the open event at line 127), this appends a string to the binary response, corrupting the downloaded file. When headers are already sent, destroy the response instead.
Suggested fix
readStream.on('error', (err) => {
if (!res.headersSent) {
res.writeHead(500);
res.end('Failed to read file');
return;
}
res.destroy(err);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| readStream.on('error', () => { | |
| if (!res.headersSent) { | |
| res.writeHead(500); | |
| } | |
| res.end('Failed to read file'); | |
| }); | |
| readStream.on('error', (err) => { | |
| if (!res.headersSent) { | |
| res.writeHead(500); | |
| res.end('Failed to read file'); | |
| return; | |
| } | |
| res.destroy(err); | |
| }); |
🤖 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 `@packages/desktop/src/main/preview-server.ts` around lines 121 - 126, The
readStream error handler currently always calls res.end('Failed to read file'),
which corrupts binary responses when headers have already been sent; update the
readStream.on('error') handler (the listener attached to readStream) to check
res.headersSent first: if headers have not been sent, write a 500 status and
call res.end with the error message, otherwise call res.destroy() (or res.end()
without writing additional body) to terminate the connection; ensure the code
references the existing readStream.on('error') listener and uses
res.headersSent, res.writeHead, and res.destroy to avoid appending text to an
in-progress binary response.
💡 What: Refactored
packages/desktop/src/main/preview-server.tsto replace synchronous file operations (fs.existsSync,fs.statSync,fs.readFileSync) with their asynchronous counterparts (fs.promises.access,fs.promises.stat,fs.createReadStream).🎯 Why: The preview server runs in the Electron main process. Using synchronous operations blocks the main Node.js event loop. This leads to poor concurrency, hangs, and UI micro-stuttering, especially when serving large or numerous static assets (like JS bundles, videos, or images). Also, reading large files into memory using
fs.readFileSynccreates unnecessary memory overhead.📊 Impact:
fs.createReadStream().pipe().🔬 Measurement:
Run the desktop app (
pnpm devinpackages/desktop), start a preview server for a large build output folder containing numerous JS chunks or large media files. Open the Network tab and observe the concurrent requests no longer stalling each other. CPU and Memory profiling in DevTools will show a smoothed out event loop with fewer large memory spikes during file access.PR created automatically by Jules for task 11167938262434567867 started by @iotserver24
Summary by CodeRabbit