Release v4.0.0 #6
Conversation
BREAKING CHANGE: GitKeyKitCodes enum changed to string literals - Replace numeric error codes with string literals for better debugging - Implement consistent error handling across all modules - Add proper error propagation chain - Improve error messages and debugging information - Add detailed error logging - Centralize error handling in CLI layer
# [4.0.0-next.1](v3.0.0...v4.0.0-next.1) (2025-01-14) ### Code Refactoring * **error:** implement consistent error handling system ([8857fde](8857fde)) ### Features * add input validation for key import ([e6aa7a9](e6aa7a9)) ### BREAKING CHANGES * **error:** GitKeyKitCodes enum changed to string literals - Replace numeric error codes with string literals for better debugging - Implement consistent error handling across all modules - Add proper error propagation chain - Improve error messages and debugging information - Add detailed error logging - Centralize error handling in CLI layer
WalkthroughThis pull request introduces a comprehensive refactoring of error handling and function signatures across multiple files in the GitKeyKit project. The changes focus on transitioning from using numeric error codes to a more robust error handling approach using a custom Changes
Sequence DiagramsequenceDiagram
participant CLI as Command Line Interface
participant Main as Main Function
participant SystemCheck as System Check
participant Commands as GitKeyKit Commands
participant Error as Error Handler
CLI->>Main: Invoke command
Main->>SystemCheck: Check dependencies
alt Dependencies not met
SystemCheck-->>Error: Throw GitKeyKitError
Error-->>CLI: Display error message
CLI->>CLI: Exit with error
else Dependencies met
Main->>Commands: Execute command
alt Command fails
Commands-->>Error: Throw GitKeyKitError
Error-->>CLI: Display error message
CLI->>CLI: Exit with error
else Command succeeds
Commands->>CLI: Complete operation
end
end
Poem
Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (20)
src/utils/checkSecretKeys.ts (1)
13-40: Refactor to simplify promise handling and avoid the Promise constructor anti-patternCurrently, the function wraps callback-based code in a
new Promise, which can be simplified usingutil.promisifyandexec. This refactoring enhances readability and maintainability.Apply this refactor to simplify the function:
-import { spawn } from "child_process"; +import { exec } from "child_process"; +import { promisify } from "util"; const logger = createLogger("utils:checkSecretKeys"); -export async function checkSecretKeys(): Promise<void> { - return new Promise((resolve, reject) => { - const gpgProcess = spawn("gpg", ["--list-secret-keys"]); - let output: string = ""; - - gpgProcess.stdout.on("data", (data: Buffer) => { - output += data.toString(); - }); - - gpgProcess.on("error", (error) => { - reject(new GitKeyKitError("Failed to check GPG secret keys", GitKeyKitCodes.NO_SECRET_KEYS, error)); - }); - - gpgProcess.on("close", (code) => { - if (code !== 0) { - reject(new GitKeyKitError("GPG process exited with non-zero code", GitKeyKitCodes.NO_SECRET_KEYS, { exitCode: code })); - return; - } - - if (!output.includes("sec")) { - reject(new GitKeyKitError("No GPG secret keys found", GitKeyKitCodes.NO_SECRET_KEYS)); - return; - } - - logger.debug("Found existing GPG secret keys"); - resolve(); - }); - }); +const execAsync = promisify(exec); + +export async function checkSecretKeys(): Promise<void> { + try { + const { stdout } = await execAsync("gpg --list-secret-keys"); + if (!stdout.includes("sec")) { + throw new GitKeyKitError("No GPG secret keys found", GitKeyKitCodes.NO_SECRET_KEYS); + } + logger.debug("Found existing GPG secret keys"); + } catch (error) { + throw new GitKeyKitError("Failed to check GPG secret keys", GitKeyKitCodes.NO_SECRET_KEYS, error); + } }src/commands/import.ts (3)
17-20: Use asynchronous file reading to prevent blocking the event loopUsing
readFileSyncblocks the event loop, which can negatively impact performance in an asynchronous context. Consider usingfs.promises.readFilefor non-blocking I/O operations.Apply this refactor:
-import { readFileSync } from "fs"; +import { promises as fs } from "fs"; ... - keyContent = readFileSync(keyPath, "utf-8"); + keyContent = await fs.readFile(keyPath, "utf-8");
27-35: Refactor to use asynchronousspawnfor non-blocking executionUsing
execSyncblocks the event loop. Refactor to usespawnin an asynchronous manner to handle the GPG import process without blocking.Apply this refactor:
-import { execSync } from "child_process"; +import { spawn } from "child_process"; ... - execSync("gpg --import", { - input: keyContent, - stdio: ["pipe", "inherit", "inherit"], - }); + await new Promise<void>((resolve, reject) => { + const gpgImport = spawn("gpg", ["--import"], { + stdio: ["pipe", "inherit", "inherit"], + }); + gpgImport.stdin.write(keyContent); + gpgImport.stdin.end(); + + gpgImport.on("close", (code) => { + if (code === 0) { + logger.green("GPG key imported successfully"); + resolve(); + } else { + reject(new GitKeyKitError("Failed to import GPG key", GitKeyKitCodes.KEY_IMPORT_ERROR, { exitCode: code })); + } + }); + + gpgImport.on("error", (error) => { + reject(new GitKeyKitError("Failed to start GPG import process", GitKeyKitCodes.KEY_IMPORT_ERROR, error)); + }); + });
36-40: Simplify error handling by removing unnecessary nestedtry...catchblocksThe nested
try...catchblocks can be flattened to improve readability and maintainability. Since you're usingasync/await, you can handle errors in a singletry...catch.Apply this refactor:
export async function importKey(keyPath: string): Promise<void> { - try { - let keyContent: string; - try { - keyContent = await fs.readFile(keyPath, "utf-8"); - } catch (error) { - throw new GitKeyKitError(`Failed to read key file: ${keyPath}`, GitKeyKitCodes.KEY_IMPORT_ERROR, error); - } + try { + const keyContent = await fs.readFile(keyPath, "utf-8"); if (!keyContent.includes("-----BEGIN PGP PRIVATE KEY BLOCK-----")) { throw new GitKeyKitError("Invalid key file format: Missing PGP private key block", GitKeyKitCodes.KEY_IMPORT_ERROR); } await new Promise<void>((resolve, reject) => { // Spawn code here... }); - } catch (error) { - if (error instanceof GitKeyKitError) { - throw error; - } - throw new GitKeyKitError("Unexpected error during key import", GitKeyKitCodes.KEY_IMPORT_ERROR, error); - } + } catch (error) { + throw error instanceof GitKeyKitError + ? error + : new GitKeyKitError("Unexpected error during key import", GitKeyKitCodes.KEY_IMPORT_ERROR, error); + } }src/utils/createKey.ts (1)
26-43: Consider refactoring to use a promise-based approach without thenew PromiseconstructorUsing
new Promisewith callbacks within anasyncfunction can be refactored for better readability. Sincespawndoesn't natively return a promise, you can wrap it using utility functions or use existing libraries that support promises.While interacting with a process that requires user input makes it challenging to fully abstract away callbacks, you can still enhance the code structure. For example, you might create a helper function to promisify the
spawnprocess.src/systemCheck.ts (3)
28-28: Clarify error code for command execution failureIn the
catchblock, you're throwing aGitKeyKitErrorwith the codeGPG_NOT_FOUND. If the error is due to issues other than GPG not being found (e.g., permission issues), consider using a more general error code or creating a new one to reflect execution failures accurately.
40-40: Clarify error code for Git command execution failureSimilar to the previous comment on GPG, in
checkGitInstallation, you throw aGitKeyKitErrorwith the codeGIT_NOT_FOUNDin thecatchblock. If the error arises from reasons other than Git not being installed, it might be helpful to differentiate the error codes for clearer debugging.
57-61: Consider more specific error code for dependency check failureWhen handling errors in
checkRequiredDependencies, you throw aGitKeyKitErrorwith the codeINVALID_INPUTif the error is not an instance ofGitKeyKitError. Since the failure is related to dependency checks, consider using a more specific error code likeDEPENDENCY_CHECK_FAILEDto improve error tracing.src/commands/reset.ts (4)
10-13: Consistent naming convention for functionThe function
restoreGpgConfigis correctly converted to an asynchronous function returningPromise<void>. However, ensure that the function name follows consistent casing conventions throughout the codebase. Elsewhere, 'GPG' is capitalized; consider renaming it torestoreGPGConfigfor consistency.
35-36: Consistent function namingThe function
clearGpgConfigshould follow the same naming convention asrestoreGpgConfig. For consistency, consider renaming it toclearGPGConfig.
41-53: Handle case wheregpg.confdoes not existWhen clearing the GPG configuration, if
gpg.confdoes not exist,writeFileSyncwill create a new empty file. If the intention is to remove existing configurations, this is acceptable. However, if the file does not exist, perhaps a log message indicating that there's nothing to clear would be helpful.Consider adding a check:
+ if (!existsSync(gpgConfPath)) { + logger.log("No GPG configuration file to clear."); + return; + }
88-91: Consistent error codes for reset failuresIn the
catchblock of theresetfunction, any non-GitKeyKitErrorexceptions are wrapped in aGitKeyKitErrorwith the codeGIT_CONFIG_RESET_ERROR. If the error originates from resetting the GPG configuration, this code might be misleading. Consider using a more general error code or differentiating between Git and GPG reset errors.src/utils/linuxConfig.ts (2)
33-39: Simplify error handling increateDirectoryfunctionWhen using
mkdirwith{ recursive: true }, theEEXISTerror is handled internally and does not throw an exception. Therefore, the check forEEXISTin the catch block is unnecessary and can be removed to simplify the code.Apply this diff to simplify the error handling:
async function createDirectory(path: string): Promise<void> { try { await mkdir(path, { mode: 0o700, recursive: true }); logger.debug(`Created directory: ${path}`); } catch (error) { - if ((error as { code?: string }).code !== "EEXIST") { throw new GitKeyKitError(`Failed to create directory: ${path}`, GitKeyKitCodes.HOME_DIR_NOT_FOUND, error); - } } }
Line range hint
85-92: Use logger instead ofconsole.logfor consistent loggingTo maintain consistent logging practices across the application, replace
console.logwithlogger.logor an appropriate logging level when displaying messages.Apply this diff to use the logger for output:
- console.log( + logger.log( boxen( chalk.white( [ "Changes written to GPG config.", "(~/.gnupg/gpg.conf)", "", "> use-agent", "> pinentry-mode loopback", ].join("\n") ), { padding: 1, margin: 1, borderStyle: "round", } ) );src/utils/setGitConfig.ts (6)
47-49: Optimize the isPrimaryKey flag reset logic.The
isPrimaryKeyflag is reset for everyssbentry, which is unnecessary since webreakafter finding the first primary key's fingerprint. Consider moving this reset logic inside theif (isPrimaryKey && parts[0] === "fpr")block.- if (parts[0] === "ssb") { - isPrimaryKey = false; - }
56-56: Enhance debug logging with more context.Consider adding more context to the debug log, such as whether it's a primary or subkey.
- logger.debug(`Found GPG key: ${keyFingerprint}`); + logger.debug(`Found primary GPG key fingerprint: ${keyFingerprint}`);
66-72: Add debug logging for better observability.Consider adding debug logging to track Git config changes.
async function setGitConfigValue(key: string, value: string): Promise<void> { try { + logger.debug(`Setting git config ${key}=${value}`); await execFileAsync("git", ["config", "--global", key, value]); + logger.debug(`Successfully set git config ${key}`); } catch (error) { throw new GitKeyKitError(`Error setting git config ${key}`, GitKeyKitCodes.GIT_CONFIG_ERROR, error); } }
82-86: Strengthen email validation regex.The current email regex is basic and might allow some invalid email formats. Consider using a more comprehensive regex pattern.
- const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + const emailRegex = /^[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$/;
93-99: Consider defining configs as a constant.The Git config entries array could be defined as a constant outside the function for better maintainability and reusability.
+const GIT_CONFIG_ENTRIES = [ + ["user.name", ""], + ["user.email", ""], + ["user.signingkey", ""], + ["commit.gpgsign", "true"], + ["tag.gpgsign", "true"], + ["gpg.program", ""] +] as const; + export async function setGitConfig(gpgPath: string): Promise<void> { // ... - const configs = [ - ["user.name", username], - ["user.email", email], - ["user.signingkey", keyFingerprint], - ["commit.gpgsign", "true"], - ["tag.gpgsign", "true"], - ["gpg.program", gpgPath], - ]; + const configs = GIT_CONFIG_ENTRIES.map(([key, defaultValue]) => { + switch (key) { + case "user.name": return [key, username]; + case "user.email": return [key, email]; + case "user.signingkey": return [key, keyFingerprint]; + case "gpg.program": return [key, gpgPath]; + default: return [key, defaultValue]; + } + });
104-107: Enhance error context in the config loop.The error message could include both the key and the attempted value for better debugging.
- throw new GitKeyKitError(`Failed to set git config: ${key}`, GitKeyKitCodes.GIT_CONFIG_ERROR, error); + throw new GitKeyKitError( + `Failed to set git config ${key}=${value}`, + GitKeyKitCodes.GIT_CONFIG_ERROR, + error + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
bin/index.ts(2 hunks)package.json(1 hunks)src/commands/import.ts(1 hunks)src/commands/reset.ts(1 hunks)src/commands/start.ts(1 hunks)src/gitkeykitCodes.ts(1 hunks)src/systemCheck.ts(1 hunks)src/utils/checkDependencies.ts(0 hunks)src/utils/checkSecretKeys.ts(1 hunks)src/utils/createKey.ts(1 hunks)src/utils/linuxConfig.ts(2 hunks)src/utils/setGitConfig.ts(2 hunks)
💤 Files with no reviewable changes (1)
- src/utils/checkDependencies.ts
✅ Files skipped from review due to trivial changes (1)
- package.json
🔇 Additional comments (7)
src/systemCheck.ts (1)
22-25: Question on necessity of checkinggpgPathafter command executionAfter executing
COMMANDS.GPG_CHECK, is it necessary to check ifgpgPathis empty? Ifgpgis not installed, theexecAsyncshould throw an error, which is already handled in thecatchblock. The additional check for an emptygpgPathmay be redundant.src/gitkeykitCodes.ts (2)
1-14: Well-structured custom error classThe
GitKeyKitErrorclass is well-defined, extending theErrorclass appropriately. Using thepublickeyword in the constructor parameters efficiently assigns properties. Good job on setting the prototype to maintain the correct inheritance chain.
47-62: Efficient error message mappingUsing an object to map error codes to messages in
getErrorMessageenhances readability and maintainability. This approach simplifies the retrieval of error messages compared to a switch statement.src/commands/reset.ts (1)
84-85: Restore GPG configuration on non-Linux platformsCurrently,
restoreGpgConfigis only called on Linux platforms. If the application may run on macOS or other UNIX-like systems, consider whether GPG configuration restoration is necessary on those platforms as well.Would you like to extend the platform check or clarify in documentation why it's limited to Linux?
src/utils/linuxConfig.ts (1)
43-48: Good use of custom error handling inappendToFileThe
appendToFilefunction now throws aGitKeyKitErrorwith a specific error code when file operations fail, enhancing error reporting and consistency across the codebase.src/commands/start.ts (1)
12-38: Well-structured error handling and consistent use ofGitKeyKitErrorThe
startfunction now consistently throwsGitKeyKitErrorinstances for error scenarios, which enhances error propagation and simplifies error handling in the calling functions. The sequential asynchronous calls are correctly awaited, ensuring that each step completes before proceeding.src/utils/setGitConfig.ts (1)
1-7: LGTM! Clean import organization and proper logger setup.The imports are well-organized, and the logger setup follows the module pattern consistently.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
README.md (2)
27-27: Consider adding key file format details.The import command example uses a generic filename. Consider adding a note about the expected key file format to help users prepare their keys correctly.
-gitkeykit import my_key.txt +gitkeykit import my_key.txt # Accepts ASCII-armored PGP key files
57-58: Consider expanding contribution guidelines.While the welcoming message is good, consider adding:
- Link to contribution guidelines
- Development setup instructions
- Code of conduct reference
## 🤝 Contributing -Contributions are welcome! Please feel free to submit a Pull Request. +Contributions are welcome! Check out our [contribution guidelines](CONTRIBUTING.md) to get started. + +1. Fork the repository +2. Create your feature branch +3. Follow our development setup guide +4. Ensure tests pass +5. Submit a pull request + +Please note that this project follows our [Code of Conduct](CODE_OF_CONDUCT.md).🧰 Tools
🪛 LanguageTool
[style] ~58-~58: The phrase ‘feel free to’ is used quite frequently. Consider using a less frequent alternative to set your writing apart from others and make it sound more professional.
Context: ...Contributing Contributions are welcome! Please feel free to submit a Pull Request.(FEEL_FREE_TO_STYLE_ME)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~58-~58: The phrase ‘feel free to’ is used quite frequently. Consider using a less frequent alternative to set your writing apart from others and make it sound more professional.
Context: ...Contributing Contributions are welcome! Please feel free to submit a Pull Request.
(FEEL_FREE_TO_STYLE_ME)
🔇 Additional comments (3)
README.md (3)
9-16: LGTM! Clear installation instructions.The installation section is well-structured, providing both the recommended npx usage and global installation option.
39-43: LGTM! Comprehensive command documentation.The command options section clearly lists all available commands with their descriptions.
45-54: LGTM! Well-structured features section.The features section effectively communicates the tool's capabilities with a clear hierarchical structure.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
README.md (3)
26-27: Inconsistency in import command documentation.The command example doesn't match the command option format. For consistency:
-gitkeykit import my_key.txt +gitkeykit --import my_key.txt
39-43: Enhance command options documentation.Consider adding descriptions of expected arguments and example values:
### Command Options - `--reset` Reset Git and GPG configurations - `--help` Show help information - `--version` Show version number - - `--import <key_path.txt>` Import and configure PGP key from file + - `--import <key_path.txt>` Import and configure PGP key from file (e.g., my_key.txt, private_key.asc)
57-58: Consider enhancing the contributing section.The contributing section could benefit from more detailed guidance.
## 🤝 Contributing -Contributions are welcome! Please feel free to submit a Pull Request. +Contributions are welcome! Here's how you can help: + +1. Fork the repository +2. Create your feature branch (`git checkout -b feature/amazing-feature`) +3. Commit your changes (`git commit -m 'Add amazing feature'`) +4. Push to the branch (`git push origin feature/amazing-feature`) +5. Open a Pull Request🧰 Tools
🪛 LanguageTool
[style] ~58-~58: The phrase ‘feel free to’ is used quite frequently. Consider using a less frequent alternative to set your writing apart from others and make it sound more professional.
Context: ...Contributing Contributions are welcome! Please feel free to submit a Pull Request.(FEEL_FREE_TO_STYLE_ME)
bin/index.ts (3)
4-7: Consider grouping related imports.For better organization, consider grouping related imports:
import arg from "arg"; import chalk from "chalk"; +import boxen from "boxen"; + import { fileURLToPath } from "url"; import { dirname, join } from "path"; import { readFileSync } from "fs"; -import boxen from "boxen";
103-120: Consider adding error codes to console output for better debugging.The error messages in the console could be more helpful for users reporting issues.
- logger.error(`Invalid argument: ${error.message}`); + logger.error(`Invalid argument (${error.code}): ${error.message}`); console.log("------"); usage(); process.exit(1); - logger.error("An unexpected error occurred:", error); + const errorCode = error.code || 'UNKNOWN_ERROR'; + logger.error(`An unexpected error occurred (${errorCode}):`, error);
124-127: Consider adding process name to fatal error logs.For better error tracking in process managers:
main().catch((error) => { - logger.error("Fatal error:", error); + logger.error("[gitkeykit] Fatal error:", error); process.exit(1); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(1 hunks)bin/index.ts(2 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~58-~58: The phrase ‘feel free to’ is used quite frequently. Consider using a less frequent alternative to set your writing apart from others and make it sound more professional.
Context: ...Contributing Contributions are welcome! Please feel free to submit a Pull Request.
(FEEL_FREE_TO_STYLE_ME)
🔇 Additional comments (1)
bin/index.ts (1)
41-52: LGTM! Improved error handling in handleImport.The error handling has been enhanced with proper error wrapping and type checking. The previous issue with missing
awaithas been fixed.
|
🎉 This PR is included in version 4.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Promise<void>.GitKeyKitError.Chores
4.0.0-next.1.