Skip to content

Commit cd26983

Browse files
committed
feat(core): enhance error handling and robustness 🛡️
1 parent 2a10865 commit cd26983

3 files changed

Lines changed: 194 additions & 54 deletions

File tree

src/GitSmart.js

Lines changed: 88 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -48,48 +48,79 @@ class GitSmart {
4848
}
4949

5050
async validateEnvironment() {
51+
// Check if we're in a git repository
5152
if (!GitUtils.isGitRepository()) {
5253
throw new Error('Not a git repository. Please run this command from within a git repository.')
5354
}
55+
56+
// Check for staged changes
5457
if (!GitUtils.hasStagedChanges()) {
5558
throw new Error('No staged changes found. Please stage your changes with "git add" first.')
5659
}
57-
// Verbose logging removed for linting compliance
60+
61+
// Additional validation: check if git is accessible
62+
try {
63+
GitUtils.getStagedFiles()
64+
} catch (error) {
65+
throw new Error(`Git repository validation failed: ${error.message}`)
66+
}
5867
}
5968

6069
async analyzeChanges() {
61-
// Verbose logging removed for linting compliance
62-
const diff = GitUtils.getStagedDiff()
63-
const stagedFiles = GitUtils.getStagedFiles()
64-
const stats = GitUtils.getDiffStats()
65-
const analysis = this.diffAnalyzer.analyze(diff, stagedFiles)
66-
analysis.stats = stats
67-
// Verbose logging removed for linting compliance
68-
return analysis
70+
try {
71+
const diff = GitUtils.getStagedDiff()
72+
const stagedFiles = GitUtils.getStagedFiles()
73+
const stats = GitUtils.getDiffStats()
74+
75+
// Validate that we have some changes to analyze
76+
if (!diff && stagedFiles.length === 0) {
77+
throw new Error('No changes detected to analyze')
78+
}
79+
80+
const analysis = this.diffAnalyzer.analyze(diff, stagedFiles)
81+
analysis.stats = stats
82+
83+
return analysis
84+
} catch (error) {
85+
throw new Error(`Failed to analyze changes: ${error.message}`)
86+
}
6987
}
7088

7189
async getStyleGuide() {
72-
// Verbose logging removed for linting compliance
73-
const commits = GitUtils.getRecentCommits(50)
74-
const historyAnalysis = this.historyAnalyzer.analyzeCommitHistory(commits)
75-
76-
// v1.1.0: Use enhanced style guide generation
77-
const styleGuide = this.options.enhanced
78-
? this.historyAnalyzer.generateEnhancedStyleGuide(historyAnalysis)
79-
: this.historyAnalyzer.generateStyleGuide(historyAnalysis)
80-
81-
// Verbose logging removed for linting compliance
82-
// Add the adaptation method
83-
styleGuide.adaptMessageToStyle = (message, guide) => {
84-
return this.historyAnalyzer.adaptMessageToStyle(message, guide)
85-
}
86-
87-
// v1.1.0: Store enhanced analysis for potential CLI options
88-
if (this.options.enhanced) {
89-
styleGuide.enhancedAnalysis = historyAnalysis
90+
try {
91+
const commits = GitUtils.getRecentCommits(50)
92+
93+
// Validate commit history
94+
if (!Array.isArray(commits)) {
95+
throw new Error('Invalid commit history format')
96+
}
97+
98+
const historyAnalysis = this.historyAnalyzer.analyzeCommitHistory(commits)
99+
100+
// v1.1.0: Use enhanced style guide generation
101+
const styleGuide = this.options.enhanced
102+
? this.historyAnalyzer.generateEnhancedStyleGuide(historyAnalysis)
103+
: this.historyAnalyzer.generateStyleGuide(historyAnalysis)
104+
105+
// Validate style guide
106+
if (!styleGuide || typeof styleGuide !== 'object') {
107+
throw new Error('Failed to generate style guide')
108+
}
109+
110+
// Add the adaptation method
111+
styleGuide.adaptMessageToStyle = (message, guide) => {
112+
return this.historyAnalyzer.adaptMessageToStyle(message, guide)
113+
}
114+
115+
// v1.1.0: Store enhanced analysis for potential CLI options
116+
if (this.options.enhanced) {
117+
styleGuide.enhancedAnalysis = historyAnalysis
118+
}
119+
120+
return styleGuide
121+
} catch (error) {
122+
throw new Error(`Failed to analyze commit history: ${error.message}`)
90123
}
91-
92-
return styleGuide
93124
}
94125

95126
generateCommitMessages(analysis, styleGuide) {
@@ -136,23 +167,44 @@ class GitSmart {
136167
}
137168

138169
async executeCommit(message) {
139-
if (!(message)) {
140-
// Info message removed for linting compliance
170+
if (!message) {
141171
return
142172
}
173+
143174
if (this.options.dryRun) {
144-
// Dry run output removed for linting compliance
145175
return
146176
}
177+
178+
// Additional validation before committing
179+
if (typeof message !== 'string' || message.trim().length === 0) {
180+
throw new Error('Invalid commit message: must be a non-empty string')
181+
}
182+
183+
// Check message length (git has practical limits)
184+
if (message.length > 3600) {
185+
throw new Error('Commit message too long (maximum 3600 characters)')
186+
}
187+
147188
try {
148-
GitUtils.commit(message)
189+
const result = GitUtils.commit(message.trim())
190+
149191
if (this.interactive) {
150192
this.interactive.displaySuccess(`Committed with message: "${message}"`)
151-
} else {
152-
// Success message handled by interactive prompt
153193
}
154-
// Verbose logging removed for linting compliance
194+
195+
return result
155196
} catch (error) {
197+
// Provide more helpful error messages for common issues
198+
if (error.message.includes('nothing to commit')) {
199+
throw new Error('No changes to commit. Please stage your changes first.')
200+
}
201+
if (error.message.includes('not a git repository')) {
202+
throw new Error('Not in a git repository. Please run from within a git repository.')
203+
}
204+
if (error.message.includes('Author identity unknown')) {
205+
throw new Error('Git user not configured. Please set user.name and user.email with git config.')
206+
}
207+
156208
throw new Error(`Failed to commit: ${error.message}`)
157209
}
158210
}

src/generators/MessageGenerator.js

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -202,16 +202,41 @@ class MessageGenerator {
202202
}
203203

204204
generateMessages(analysis, styleGuide, options = {}) {
205+
// Input validation
206+
if (!analysis || typeof analysis !== 'object') {
207+
throw new Error('Analysis object is required')
208+
}
209+
if (!styleGuide || typeof styleGuide !== 'object') {
210+
throw new Error('Style guide object is required')
211+
}
212+
if (!analysis.changeType) {
213+
throw new Error('Analysis must include changeType')
214+
}
215+
205216
const messages = []
206-
// Primary message based on analysis
207-
const primary = this.generatePrimaryMessage(analysis, styleGuide)
208-
messages.push({ message: primary, confidence: analysis.confidence, type: 'primary' })
209-
// Alternative messages
210-
if (options.includeAlternatives) {
211-
const alternatives = this.generateAlternatives(analysis, styleGuide)
212-
alternatives.forEach(alt => messages.push(alt))
213-
}
214-
return messages
217+
218+
try {
219+
// Primary message based on analysis
220+
const primary = this.generatePrimaryMessage(analysis, styleGuide)
221+
222+
if (!primary || typeof primary !== 'string' || primary.trim().length === 0) {
223+
throw new Error('Failed to generate primary commit message')
224+
}
225+
226+
messages.push({ message: primary, confidence: analysis.confidence || 50, type: 'primary' })
227+
228+
// Alternative messages
229+
if (options.includeAlternatives) {
230+
const alternatives = this.generateAlternatives(analysis, styleGuide)
231+
if (Array.isArray(alternatives)) {
232+
alternatives.forEach(alt => messages.push(alt))
233+
}
234+
}
235+
236+
return messages
237+
} catch (error) {
238+
throw new Error(`Failed to generate commit messages: ${error.message}`)
239+
}
215240
}
216241

217242
generatePrimaryMessage(analysis, styleGuide) {

src/utils/git.js

Lines changed: 72 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@ const { execSync, spawnSync } = require('child_process')
33
class GitUtils {
44
static isGitRepository() {
55
try {
6-
execSync('git rev-parse --git-dir', { stdio: 'ignore' })
6+
execSync('git rev-parse --git-dir', {
7+
stdio: 'ignore',
8+
timeout: 5000 // 5 second timeout for simple check
9+
})
710
return true
811
} catch {
912
return false
@@ -12,7 +15,10 @@ class GitUtils {
1215

1316
static hasStagedChanges() {
1417
try {
15-
const output = execSync('git diff --cached --name-only', { encoding: 'utf8' })
18+
const output = execSync('git diff --cached --name-only', {
19+
encoding: 'utf8',
20+
timeout: 10000 // 10 second timeout
21+
})
1622
return output.trim().length > 0
1723
} catch {
1824
return false
@@ -21,42 +27,86 @@ class GitUtils {
2127

2228
static getStagedDiff() {
2329
try {
24-
return execSync('git diff --cached', { encoding: 'utf8' })
30+
return execSync('git diff --cached', {
31+
encoding: 'utf8',
32+
timeout: 30000 // 30 second timeout
33+
})
2534
} catch (error) {
35+
if (error.code === 'ETIMEDOUT') {
36+
throw new Error('Git diff command timed out - repository may be too large')
37+
}
2638
throw new Error('Failed to get staged changes: ' + error.message)
2739
}
2840
}
2941

3042
static getStagedFiles() {
3143
try {
32-
const output = execSync('git diff --cached --name-status', { encoding: 'utf8' })
44+
const output = execSync('git diff --cached --name-status', {
45+
encoding: 'utf8',
46+
timeout: 30000
47+
})
48+
49+
if (!output || !output.trim()) {
50+
return []
51+
}
52+
3353
return output.trim().split('\n')
3454
.filter(line => line.trim())
3555
.map(line => {
36-
const [status, ...pathParts] = line.split('\t')
56+
const parts = line.split('\t')
57+
if (parts.length < 2) {
58+
throw new Error(`Invalid git status line: ${line}`)
59+
}
60+
const [status, ...pathParts] = parts
3761
return {
3862
status: status.trim(),
3963
path: pathParts.join('\t').trim()
4064
}
4165
})
4266
} catch (error) {
67+
if (error.code === 'ETIMEDOUT') {
68+
throw new Error('Git status command timed out - repository may be too large')
69+
}
4370
throw new Error('Failed to get staged files: ' + error.message)
4471
}
4572
}
4673

4774
static getRecentCommits(limit = 50) {
75+
// Input validation
76+
if (!Number.isInteger(limit) || limit < 1 || limit > 1000) {
77+
throw new Error('Commit limit must be an integer between 1 and 1000')
78+
}
79+
4880
try {
49-
const output = execSync(`git log --oneline -${limit}`, { encoding: 'utf8' })
81+
const output = execSync(`git log --oneline -${limit}`, {
82+
encoding: 'utf8',
83+
timeout: 30000
84+
})
85+
86+
if (!output || !output.trim()) {
87+
return []
88+
}
89+
5090
return output.trim().split('\n')
5191
.filter(line => line.trim())
5292
.map(line => {
5393
const spaceIndex = line.indexOf(' ')
94+
if (spaceIndex === -1) {
95+
// Handle commit with no message (shouldn't happen but be safe)
96+
return {
97+
hash: line.trim(),
98+
message: ''
99+
}
100+
}
54101
return {
55102
hash: line.substring(0, spaceIndex),
56103
message: line.substring(spaceIndex + 1)
57104
}
58105
})
59-
} catch {
106+
} catch (error) {
107+
if (error.code === 'ETIMEDOUT') {
108+
console.warn('Git log command timed out, using empty commit history')
109+
}
60110
return []
61111
}
62112
}
@@ -95,9 +145,18 @@ class GitUtils {
95145

96146
static getDiffStats() {
97147
try {
98-
const output = execSync('git diff --cached --stat', { encoding: 'utf8' })
148+
const output = execSync('git diff --cached --stat', {
149+
encoding: 'utf8',
150+
timeout: 30000
151+
})
152+
153+
if (!output || !output.trim()) {
154+
return { files: 0, insertions: 0, deletions: 0 }
155+
}
156+
99157
const lines = output.trim().split('\n')
100158
const statsLine = lines[lines.length - 1]
159+
101160
if (statsLine && statsLine.includes('changed')) {
102161
const matches = statsLine.match(/(\d+) files? changed(?:, (\d+) insertions?\(\+\))?(?:, (\d+) deletions?\(-\))?/)
103162
if (matches) {
@@ -108,8 +167,12 @@ class GitUtils {
108167
}
109168
}
110169
}
170+
111171
return { files: 0, insertions: 0, deletions: 0 }
112-
} catch {
172+
} catch (error) {
173+
if (error.code === 'ETIMEDOUT') {
174+
console.warn('Git stat command timed out, returning zero stats')
175+
}
113176
return { files: 0, insertions: 0, deletions: 0 }
114177
}
115178
}

0 commit comments

Comments
 (0)