Improve Electron rebuild error handling#3
Conversation
MXASoundNDEv
deleted the
codex/improve-error-handling-in-rebuild-electron.js
branch
There was a problem hiding this comment.
Pull Request Overview
This PR improves error handling and robustness of the Electron rebuild script by implementing stricter failure reporting, fallback mechanisms for missing npx, and enhanced error output formatting.
- Enhanced error handling with detailed error messages and proper exit codes
- Added fallback to local cmake-js binary when npx is unavailable on POSIX systems
- Updated documentation to reflect the stricter failure reporting behavior
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| scripts/rebuild-electron.js | Complete rewrite of rebuild logic with improved error handling, fallback mechanisms, and structured command execution |
| README.md | Added documentation about stricter failure reporting and npx fallback behavior |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| const version = process.argv[2] || process.env.ELECTRON_VERSION || process.env.npm_config_target; | ||
| if (!version) { | ||
| console.error('[soem-node] Veuillez fournir la version d\'Electron (ex: 30.0.0).'); | ||
| console.error("[soem-node] Veuillez fournir la version d'Electron (ex: 30.0.0)."); |
There was a problem hiding this comment.
[nitpick] The quote character has been changed from a backtick to a straight quote, which may cause display inconsistency. Consider keeping the original quote style for consistency.
| console.error("[soem-node] Veuillez fournir la version d'Electron (ex: 30.0.0)."); | |
| console.error(`[soem-node] Veuillez fournir la version d'Electron (ex: 30.0.0).`); |
| ? ['rebuild', '--runtime=electron', `--runtimeVersion=${version}`, '--loglevel=verbose'] | ||
| : ['cmake-js', 'rebuild', '--runtime=electron', `--runtimeVersion=${version}`, '--loglevel=verbose']; | ||
| const spawnOptions = { | ||
| stdio: 'pipe', |
There was a problem hiding this comment.
Using 'pipe' stdio mode and then manually piping output may be less efficient than using 'inherit' for real-time output streaming, especially for long-running cmake operations.
| } | ||
|
|
||
| const npxArgs = ['cmake-js', 'rebuild', ...sharedArgs]; | ||
| let result = execute('npx', npxArgs); |
There was a problem hiding this comment.
[nitpick] The variable 'result' is reassigned in the fallback logic (line 87), which could make the code harder to follow. Consider using different variable names for clarity.
| let result = execute('npx', npxArgs); | ||
|
|
||
| if (result.error?.code === 'ENOENT') { | ||
| console.warn("[soem-node] 'npx' est indisponible, tentative avec le binaire local cmake-js."); | ||
| const directArgs = ['rebuild', ...sharedArgs]; | ||
| result = execute(localBinary, directArgs); | ||
| if (result.error || result.status !== 0) { | ||
| handleFailure(formatCommand(localBinary, directArgs), result); | ||
| } | ||
| process.exit(0); | ||
| } | ||
|
|
||
| if (result.error || result.status !== 0) { | ||
| handleFailure(formatCommand('npx', npxArgs), result); |
There was a problem hiding this comment.
[nitpick] The variable 'result' is reassigned in the fallback logic (line 87), which could make the code harder to follow. Consider using different variable names for clarity.
| let result = execute('npx', npxArgs); | |
| if (result.error?.code === 'ENOENT') { | |
| console.warn("[soem-node] 'npx' est indisponible, tentative avec le binaire local cmake-js."); | |
| const directArgs = ['rebuild', ...sharedArgs]; | |
| result = execute(localBinary, directArgs); | |
| if (result.error || result.status !== 0) { | |
| handleFailure(formatCommand(localBinary, directArgs), result); | |
| } | |
| process.exit(0); | |
| } | |
| if (result.error || result.status !== 0) { | |
| handleFailure(formatCommand('npx', npxArgs), result); | |
| let npxResult = execute('npx', npxArgs); | |
| if (npxResult.error?.code === 'ENOENT') { | |
| console.warn("[soem-node] 'npx' est indisponible, tentative avec le binaire local cmake-js."); | |
| const directArgs = ['rebuild', ...sharedArgs]; | |
| const directResult = execute(localBinary, directArgs); | |
| if (directResult.error || directResult.status !== 0) { | |
| handleFailure(formatCommand(localBinary, directArgs), directResult); | |
| } | |
| process.exit(0); | |
| } | |
| if (npxResult.error || npxResult.status !== 0) { | |
| handleFailure(formatCommand('npx', npxArgs), npxResult); |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68cd69fef21c83248d77943bb7a3b2ea