-
Notifications
You must be signed in to change notification settings - Fork 2.3k
feat(dev-hot-reload): streamline hot-reload process with ProcessManag… #7731
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -13,8 +13,9 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # npm run dev:watch -- [options] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { execSync } = require('child_process'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { readFileSync } = require('fs'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { execSync, spawn } = require('child_process'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { readFileSync, readdirSync } = require('fs'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const path = require('path'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Get major version from .nvmrc (e.g. v22.1.0 -> v22) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const NODE_VERSION = readFileSync('.nvmrc', 'utf8').trim().split('.')[0]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -30,7 +31,6 @@ const CONFIG = { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'packages/bruno-js/src/', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'packages/bruno-schema/src/' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ELECTRON_START_DELAY: 10, // seconds | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| NODEMON_WATCH_DELAY: 1000 // milliseconds | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -68,6 +68,40 @@ function log(level, msg) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class ProcessManager { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| constructor() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this._procs = []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this._shuttingDown = false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const sig of ['SIGINT', 'SIGTERM', 'SIGHUP']) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| process.on(sig, () => this.shutdown(sig)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| register(proc, name) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this._procs.push({ proc, name }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| proc.on('close', code => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!this._shuttingDown && code !== null && code !== 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| log(LOG_LEVELS.ERROR, `Process "${name}" exited unexpectedly (code ${code}), shutting down...`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+81
to
+84
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this._procs.push({ proc, name }); | |
| proc.on('close', code => { | |
| if (!this._shuttingDown && code !== null && code !== 0) { | |
| log(LOG_LEVELS.ERROR, `Process "${name}" exited unexpectedly (code ${code}), shutting down...`); | |
| this._procs.push({ proc, name }); | |
| proc.on('error', err => { | |
| if (!this._shuttingDown) { | |
| log(LOG_LEVELS.ERROR, `Process "${name}" failed to start or crashed with an error: ${err.message}`); | |
| process.exitCode = 1; | |
| this.shutdown('child-error'); | |
| } | |
| }); | |
| proc.on('close', code => { | |
| if (!this._shuttingDown && code !== null && code !== 0) { | |
| log(LOG_LEVELS.ERROR, `Process "${name}" exited unexpectedly (code ${code}), shutting down...`); | |
| process.exitCode = 1; |
Copilot
AI
Apr 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shutdown() always force-exits with status 0 after the timeout, even when shutdown was triggered by an unexpected child exit. This makes it harder for callers/scripts to detect startup failures. Consider tracking a desired exit code (e.g. 1 on 'child-exit' / spawn errors) and using that in process.exit(...).
Copilot
AI
Apr 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spawnWithPrefix splits each stdout/stderr chunk by \n and re-adds newlines. Stream chunk boundaries are arbitrary, so this can break lines (partial lines get treated as complete) and interleave prefixes incorrectly. Consider buffering per-stream until a newline is seen (or use readline on the stream) so prefixes are applied to complete lines only.
| // Spawn a process and prefix every line of its output with [name] | |
| function spawnWithPrefix(cmd, args, name, color) { | |
| const prefix = `${color}[${name}]${COLORS.nc} `; | |
| const proc = spawn(cmd, args, { stdio: ['ignore', 'pipe', 'pipe'] }); | |
| proc.stdout.on('data', data => | |
| data.toString().split('\n').filter(Boolean).forEach(line => | |
| process.stdout.write(prefix + line + '\n') | |
| ) | |
| ); | |
| proc.stderr.on('data', data => | |
| data.toString().split('\n').filter(Boolean).forEach(line => | |
| process.stderr.write(prefix + line + '\n') | |
| ) | |
| ); | |
| function attachPrefixedOutput(stream, output, prefix) { | |
| let buffer = ''; | |
| stream.on('data', data => { | |
| buffer += data.toString(); | |
| let newlineIndex = buffer.indexOf('\n'); | |
| while (newlineIndex !== -1) { | |
| let line = buffer.slice(0, newlineIndex); | |
| if (line.endsWith('\r')) { | |
| line = line.slice(0, -1); | |
| } | |
| output.write(prefix + line + '\n'); | |
| buffer = buffer.slice(newlineIndex + 1); | |
| newlineIndex = buffer.indexOf('\n'); | |
| } | |
| }); | |
| stream.on('end', () => { | |
| if (buffer.length > 0) { | |
| output.write(prefix + buffer + '\n'); | |
| } | |
| }); | |
| } | |
| // Spawn a process and prefix every line of its output with [name] | |
| function spawnWithPrefix(cmd, args, name, color) { | |
| const prefix = `${color}[${name}]${COLORS.nc} `; | |
| const proc = spawn(cmd, args, { stdio: ['ignore', 'pipe', 'pipe'] }); | |
| attachPrefixedOutput(proc.stdout, process.stdout, prefix); | |
| attachPrefixedOutput(proc.stderr, process.stderr, prefix); |
Copilot
AI
Apr 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
workspaceFlag is built from pkg.dir (an absolute filesystem path). npm workspaces typically expect a workspace name (e.g. @usebruno/common) or a repo-relative path (e.g. packages/bruno-common). If npm doesn't recognize the absolute path, it can fall back to the root watch script (which exists) and recursively re-run this script. Use --workspace=${pkg.name} or --workspace=${path.relative(rootDir, pkg.dir)} instead.
| const workspaceFlag = `--workspace=${pkg.dir}`; | |
| const workspaceFlag = `--workspace=${pkg.name}`; |
Copilot
AI
Apr 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The promise used to detect watcher readiness only resolves when output contains created . If the watch process exits early (e.g. rollup config error) before printing that line, this promise never resolves/rejects and startup hangs until the forced shutdown timeout. Consider rejecting on close (non-zero) and/or adding a readiness timeout with a clear error message.
| return new Promise(resolve => { | |
| const onData = data => { | |
| if (data.toString().includes('created ')) { | |
| proc.stdout.off('data', onData); | |
| proc.stderr.off('data', onData); | |
| log(LOG_LEVELS.SUCCESS, `${shortName} — initial watch build done`); | |
| resolve(); | |
| } | |
| }; | |
| proc.stdout.on('data', onData); | |
| proc.stderr.on('data', onData); | |
| return new Promise((resolve, reject) => { | |
| let isReady = false; | |
| const readinessTimeoutMs = 30000; | |
| const cleanup = () => { | |
| clearTimeout(readinessTimeout); | |
| proc.stdout.off('data', onData); | |
| proc.stderr.off('data', onData); | |
| proc.off('close', onClose); | |
| proc.off('error', onError); | |
| }; | |
| const onData = data => { | |
| if (data.toString().includes('created ')) { | |
| isReady = true; | |
| cleanup(); | |
| log(LOG_LEVELS.SUCCESS, `${shortName} — initial watch build done`); | |
| resolve(); | |
| } | |
| }; | |
| const onClose = code => { | |
| if (!isReady) { | |
| cleanup(); | |
| reject(new Error(`Watch failed for ${shortName} before readiness${code !== null ? ` (exit ${code})` : ''}`)); | |
| } | |
| }; | |
| const onError = err => { | |
| if (!isReady) { | |
| cleanup(); | |
| reject(err); | |
| } | |
| }; | |
| const readinessTimeout = setTimeout(() => { | |
| if (!isReady) { | |
| cleanup(); | |
| reject(new Error(`Timed out waiting for ${shortName} watch readiness after ${readinessTimeoutMs}ms`)); | |
| } | |
| }, readinessTimeoutMs); | |
| proc.stdout.on('data', onData); | |
| proc.stderr.on('data', onData); | |
| proc.on('close', onClose); | |
| proc.on('error', onError); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential hang if watcher output never contains "created ".
This assumes all watchers are rollup-based and emit "created ". If a package uses a different bundler or fails silently, this promise never resolves. Consider adding a timeout or a configurable ready signal.
🔧 Suggested timeout guard
if (pkg.hasWatch) {
const proc = procManager.register(
spawnWithPrefix('npm', ['run', 'watch', workspaceFlag], shortName, nextWatcherColor()),
shortName
);
- return new Promise(resolve => {
+ return new Promise((resolve, reject) => {
+ const timeout = setTimeout(() => {
+ reject(new Error(`${shortName} — timed out waiting for initial build`));
+ }, 60000);
const onData = data => {
if (data.toString().includes('created ')) {
proc.stdout.off('data', onData);
proc.stderr.off('data', onData);
+ clearTimeout(timeout);
log(LOG_LEVELS.SUCCESS, `${shortName} — initial watch build done`);
resolve();
}
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/dev-hot-reload.js` around lines 332 - 343, The promise waiting for a
watcher ready signal (the new Promise that registers onData on
proc.stdout/proc.stderr and resolves when data.includes('created ')) can hang if
the output never contains "created "; add a timeout guard and optional
configurable ready signal: when creating the promise in
scripts/dev-hot-reload.js, accept/configure a timeoutMs and/or readyPattern (env
var or parameter), start a timer that will reject or resolve (choose behavior)
after timeoutMs, and ensure both the timer and the stdout/stderr listeners
(onData) are cleaned up in all code paths; update references to proc, onData,
log and LOG_LEVELS.SUCCESS/shortName so the handler clears listeners and timer
before resolving on match or finishing on timeout.
Copilot
AI
Apr 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
startDevelopment is declared async and can throw/reject (e.g. cycle detection in topoSort, failed one-time builds). However the current entrypoint doesn't await it, so failures can become unhandled promise rejections instead of being caught by the outer main().catch(...). Either await startDevelopment() in the entrypoint or make startDevelopment non-async and handle errors internally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SIGHUPis not supported on Windows.Per the coding guidelines, avoid Unix-only signals without Windows fallbacks.
SIGHUPwill fail silently or error on Windows. Consider conditionally registering it:🛠️ Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents