Skip to content

Commit 386ce36

Browse files
ghostwriternrghostwriternr
committed
Fix GitManager to return valid ErrorCode enum values
GitManager.determineErrorCode() was returning custom strings like 'REPO_NOT_FOUND' that aren't in the ErrorCode enum. This caused withSession's error validation to reject them as unknown errors. Map all return values to proper ErrorCode constants.
1 parent ec04024 commit 386ce36

File tree

3 files changed

+38
-36
lines changed

3 files changed

+38
-36
lines changed

‎packages/sandbox-container/src/managers/git-manager.ts‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
* NO I/O operations - all infrastructure delegated to SessionManager via GitService
1616
*/
1717

18+
import { ErrorCode } from '@repo/shared/errors';
1819
import type { CloneOptions } from '../core/types';
1920

2021
/**
@@ -139,72 +140,72 @@ export class GitManager {
139140
}
140141

141142
/**
142-
* Determine appropriate error code based on operation and error
143+
* Determine appropriate error code based on operation and error.
144+
* Returns valid ErrorCode enum values for use with withSession error handling.
143145
*/
144146
determineErrorCode(
145147
operation: string,
146148
error: Error | string,
147149
exitCode?: number
148-
): string {
150+
): ErrorCode {
149151
const errorMessage = typeof error === 'string' ? error : error.message;
150152
const lowerMessage = errorMessage.toLowerCase();
151153

152154
// Check exit code patterns first
153155
if (exitCode === 128) {
154156
if (lowerMessage.includes('not a git repository')) {
155-
return 'NOT_A_GIT_REPO';
157+
return ErrorCode.GIT_OPERATION_FAILED;
156158
}
157159
if (lowerMessage.includes('repository not found')) {
158-
return 'REPO_NOT_FOUND';
160+
return ErrorCode.GIT_REPOSITORY_NOT_FOUND;
159161
}
160-
return 'GIT_COMMAND_ERROR';
162+
return ErrorCode.GIT_OPERATION_FAILED;
161163
}
162164

163165
// Common error patterns
164166
if (
165167
lowerMessage.includes('permission denied') ||
166168
lowerMessage.includes('access denied')
167169
) {
168-
return 'GIT_PERMISSION_DENIED';
170+
return ErrorCode.GIT_AUTH_FAILED;
169171
}
170172

171173
if (
172174
lowerMessage.includes('not found') ||
173175
lowerMessage.includes('does not exist')
174176
) {
175-
return 'GIT_NOT_FOUND';
177+
return ErrorCode.GIT_REPOSITORY_NOT_FOUND;
176178
}
177179

178180
if (lowerMessage.includes('already exists')) {
179-
return 'GIT_ALREADY_EXISTS';
181+
return ErrorCode.GIT_CLONE_FAILED;
180182
}
181183

182184
if (
183185
lowerMessage.includes('did not match') ||
184186
lowerMessage.includes('pathspec')
185187
) {
186-
return 'GIT_INVALID_REF';
188+
return ErrorCode.GIT_BRANCH_NOT_FOUND;
187189
}
188190

189191
if (
190192
lowerMessage.includes('authentication') ||
191193
lowerMessage.includes('credentials')
192194
) {
193-
return 'GIT_AUTH_FAILED';
195+
return ErrorCode.GIT_AUTH_FAILED;
194196
}
195197

196198
// Operation-specific defaults
197199
switch (operation) {
198200
case 'clone':
199-
return 'GIT_CLONE_FAILED';
201+
return ErrorCode.GIT_CLONE_FAILED;
200202
case 'checkout':
201-
return 'GIT_CHECKOUT_FAILED';
203+
return ErrorCode.GIT_CHECKOUT_FAILED;
202204
case 'getCurrentBranch':
203-
return 'GIT_BRANCH_ERROR';
204205
case 'listBranches':
205-
return 'GIT_BRANCH_LIST_ERROR';
206+
return ErrorCode.GIT_OPERATION_FAILED;
206207
default:
207-
return 'GIT_OPERATION_ERROR';
208+
return ErrorCode.GIT_OPERATION_FAILED;
208209
}
209210
}
210211

‎packages/sandbox-container/tests/managers/git-manager.test.ts‎

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { beforeEach, describe, expect, it } from 'bun:test';
2+
import { ErrorCode } from '@repo/shared/errors';
23
import { GitManager } from '@sandbox-container/managers/git-manager';
34

45
describe('GitManager', () => {
@@ -170,77 +171,77 @@ describe('GitManager', () => {
170171
});
171172

172173
describe('determineErrorCode', () => {
173-
it('should return NOT_A_GIT_REPO for exit code 128 with not a git repository message', () => {
174+
it('should return GIT_OPERATION_FAILED for exit code 128 with not a git repository message', () => {
174175
const error = new Error('fatal: not a git repository');
175176

176177
expect(manager.determineErrorCode('getCurrentBranch', error, 128)).toBe(
177-
'NOT_A_GIT_REPO'
178+
ErrorCode.GIT_OPERATION_FAILED
178179
);
179180
});
180181

181-
it('should return REPO_NOT_FOUND for exit code 128 with repository not found message', () => {
182+
it('should return GIT_REPOSITORY_NOT_FOUND for exit code 128 with repository not found message', () => {
182183
const error = new Error('fatal: repository not found');
183184

184185
expect(manager.determineErrorCode('clone', error, 128)).toBe(
185-
'REPO_NOT_FOUND'
186+
ErrorCode.GIT_REPOSITORY_NOT_FOUND
186187
);
187188
});
188189

189-
it('should return GIT_PERMISSION_DENIED for permission errors', () => {
190+
it('should return GIT_AUTH_FAILED for permission errors', () => {
190191
expect(
191192
manager.determineErrorCode('clone', new Error('Permission denied'))
192-
).toBe('GIT_PERMISSION_DENIED');
193+
).toBe(ErrorCode.GIT_AUTH_FAILED);
193194
});
194195

195-
it('should return GIT_NOT_FOUND for not found errors', () => {
196+
it('should return GIT_REPOSITORY_NOT_FOUND for not found errors', () => {
196197
expect(
197198
manager.determineErrorCode('checkout', new Error('Branch not found'))
198-
).toBe('GIT_NOT_FOUND');
199+
).toBe(ErrorCode.GIT_REPOSITORY_NOT_FOUND);
199200
});
200201

201-
it('should return GIT_INVALID_REF for pathspec errors', () => {
202+
it('should return GIT_BRANCH_NOT_FOUND for pathspec errors', () => {
202203
expect(
203204
manager.determineErrorCode(
204205
'checkout',
205206
new Error("pathspec 'branch' did not match")
206207
)
207-
).toBe('GIT_INVALID_REF');
208+
).toBe(ErrorCode.GIT_BRANCH_NOT_FOUND);
208209
});
209210

210211
it('should return GIT_AUTH_FAILED for authentication errors', () => {
211212
expect(
212213
manager.determineErrorCode('clone', new Error('Authentication failed'))
213-
).toBe('GIT_AUTH_FAILED');
214+
).toBe(ErrorCode.GIT_AUTH_FAILED);
214215
});
215216

216217
it('should return operation-specific error codes as fallback', () => {
217218
expect(
218219
manager.determineErrorCode('clone', new Error('Unknown error'))
219-
).toBe('GIT_CLONE_FAILED');
220+
).toBe(ErrorCode.GIT_CLONE_FAILED);
220221
expect(
221222
manager.determineErrorCode('checkout', new Error('Unknown error'))
222-
).toBe('GIT_CHECKOUT_FAILED');
223+
).toBe(ErrorCode.GIT_CHECKOUT_FAILED);
223224
expect(
224225
manager.determineErrorCode(
225226
'getCurrentBranch',
226227
new Error('Unknown error')
227228
)
228-
).toBe('GIT_BRANCH_ERROR');
229+
).toBe(ErrorCode.GIT_OPERATION_FAILED);
229230
expect(
230231
manager.determineErrorCode('listBranches', new Error('Unknown error'))
231-
).toBe('GIT_BRANCH_LIST_ERROR');
232+
).toBe(ErrorCode.GIT_OPERATION_FAILED);
232233
});
233234

234235
it('should handle string errors', () => {
235236
expect(manager.determineErrorCode('clone', 'repository not found')).toBe(
236-
'GIT_NOT_FOUND'
237+
ErrorCode.GIT_REPOSITORY_NOT_FOUND
237238
);
238239
});
239240

240241
it('should handle case-insensitive error matching', () => {
241242
expect(
242243
manager.determineErrorCode('clone', new Error('PERMISSION DENIED'))
243-
).toBe('GIT_PERMISSION_DENIED');
244+
).toBe(ErrorCode.GIT_AUTH_FAILED);
244245
});
245246
});
246247

‎packages/sandbox-container/tests/services/git-service.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { beforeEach, describe, expect, it, vi } from 'bun:test';
22
import type { Logger } from '@repo/shared';
3-
import type { ValidationFailedContext } from '@repo/shared/errors';
3+
import { ErrorCode, type ValidationFailedContext } from '@repo/shared/errors';
44
import type {
55
CloneOptions,
66
ServiceResult
@@ -275,7 +275,7 @@ describe('GitService', () => {
275275

276276
expect(result.success).toBe(false);
277277
if (!result.success) {
278-
expect(result.error.code).toBe('REPO_NOT_FOUND');
278+
expect(result.error.code).toBe(ErrorCode.GIT_REPOSITORY_NOT_FOUND);
279279
expect(result.error.details?.exitCode).toBe(128);
280280
expect(result.error.details?.stderr).toContain('repository not found');
281281
}
@@ -357,7 +357,7 @@ describe('GitService', () => {
357357

358358
expect(result.success).toBe(false);
359359
if (!result.success) {
360-
expect(result.error.code).toBe('GIT_INVALID_REF');
360+
expect(result.error.code).toBe(ErrorCode.GIT_BRANCH_NOT_FOUND);
361361
expect(result.error.details?.stderr).toContain('did not match');
362362
}
363363
});
@@ -448,7 +448,7 @@ describe('GitService', () => {
448448

449449
expect(result.success).toBe(false);
450450
if (!result.success) {
451-
expect(result.error.code).toBe('NOT_A_GIT_REPO');
451+
expect(result.error.code).toBe(ErrorCode.GIT_OPERATION_FAILED);
452452
expect(result.error.details?.exitCode).toBe(128);
453453
}
454454
});

0 commit comments

Comments
 (0)