Skip to content

⚡ Bolt: [performance improvement] Refactor preview server to use asynchronous I/O #89

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
iotserver24 merged 1 commit into from
May 16, 2026
Merged

⚡ Bolt: [performance improvement] Refactor preview server to use asynchronous I/O #89

Show file tree
Hide file tree
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
3 changes: 3 additions & 0 deletions .jules/bolt.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
## 2024-05-15 - Electron Main Process File Operations
**Learning:** Using synchronous `fs` methods (`readFileSync`, `existsSync`, `statSync`) in the Electron main process (e.g., `preview-server.ts`) severely blocks the Node.js event loop, preventing concurrent processing of HTTP requests and potentially causing UI thread starvation or micro-stuttering. Furthermore, buffering entire large files (e.g., `readFileSync`) consumes unnecessary heap memory.
**Action:** Always default to async file operations (`fs.promises`) and stream piping (`createReadStream(..).pipe(res)`) when dealing with HTTP response bodies in the Electron main process to maintain responsiveness and minimize memory footprint.
38 changes: 26 additions & 12 deletions packages/desktop/src/main/preview-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,15 @@ const MIME_TYPES: Record<string, string> = {
'.wasm': 'application/wasm',
};

const existsAsync = async (p: string) => {
try {
await fs.promises.access(p);
return true;
} catch {
return false;
}
};

export class PreviewServer {
private server: Server | null = null;
private port: number = 0;
Expand Down Expand Up @@ -70,13 +79,14 @@ export class PreviewServer {
return this.port ? `http://127.0.0.1:${this.port}` : '';
}

private handleRequest(req: any, res: any): void {
private async handleRequest(req: any, res: any): Promise<void> {
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 16, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

cat -n packages/desktop/src/main/preview-server.ts | head -150

Repository: 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.

const urlPath = req.url.split('?')[0];
let filePath = path.join(this.rootDir, urlPath === '/' ? 'index.html' : urlPath);

Comment on lines +82 to 85
if (!fs.existsSync(filePath)) {
// ⚡ Bolt: Replace synchronous file operations with async promises to prevent blocking the main thread
Comment on lines +82 to +86
if (!(await existsAsync(filePath))) {
const htmlPath = filePath + '.html';
if (fs.existsSync(htmlPath)) {
if (await existsAsync(htmlPath)) {
filePath = htmlPath;
} else {
res.writeHead(404);
Expand All @@ -86,10 +96,10 @@ export class PreviewServer {
}

try {
const stat = fs.statSync(filePath);
const stat = await fs.promises.stat(filePath);
if (stat.isDirectory()) {
const indexPath = path.join(filePath, 'index.html');
if (fs.existsSync(indexPath)) {
if (await existsAsync(indexPath)) {
filePath = indexPath;
} else {
res.writeHead(403);
Expand All @@ -106,16 +116,20 @@ export class PreviewServer {
const ext = path.extname(filePath).toLowerCase();
const contentType = MIME_TYPES[ext] ?? 'application/octet-stream';

try {
const content = fs.readFileSync(filePath);
// ⚡ Bolt: Use stream piping instead of readFileSync to avoid loading large files fully into memory
const readStream = fs.createReadStream(filePath);
readStream.on('error', () => {
if (!res.headersSent) {
res.writeHead(500);
}
res.end('Failed to read file');
});
Comment on lines +121 to +126
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 16, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

find . -name "preview-server.ts" -type f

Repository: iotserver24/Xibecode

Length of output: 110

🏁 Script executed:

sed -n '115,135p' ./packages/desktop/src/main/preview-server.ts

Repository: 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:

🌐 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:

🏁 Script executed:

head -n 130 ./packages/desktop/src/main/preview-server.ts | tail -n 20 | cat -n

Repository: 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.

Suggested change
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.

readStream.on('open', () => {
res.writeHead(200, {
'Content-Type': contentType,
'Cache-Control': 'no-cache',
});
res.end(content);
} catch {
res.writeHead(500);
res.end('Failed to read file');
}
});
readStream.pipe(res);
Comment on lines 129 to +133
}
}
Loading