Skip to content

Commit bfc4f60

Browse files
brandonkachenbrandonkachen
committed
refactor(sdk): convert error throwing to ErrorOr pattern
- Changed run() and validateAgents() to return ErrorOr<T> instead of throwing - Updated return types: RunResult and ValidateAgentsResult wrap results in ErrorOr - Updated all test files to unwrap ErrorOr results properly - Removed unused imports (BACKEND_URL, RunState) from client.ts - All 39 tests passing Errors now bubble up to end users through success/failure objects while maintaining backward compatibility for internal error throwing.
1 parent 038c966 commit bfc4f60

File tree

5 files changed

+191
-104
lines changed

5 files changed

+191
-104
lines changed

‎sdk/src/__tests__/run.integration.test.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ describe('Prompt Caching', () => {
2121
apiKey,
2222
})
2323
let cost1 = -1
24-
const run1 = await client.run({
24+
const run1Result = await client.run({
2525
prompt: `${filler}\n\n${prompt}`,
2626
agent: 'base',
2727
handleEvent: (event) => {
@@ -31,11 +31,15 @@ describe('Prompt Caching', () => {
3131
},
3232
})
3333

34+
if (!run1Result.success) {
35+
throw new Error(`Run1 failed: ${JSON.stringify(run1Result.error)}`)
36+
}
37+
const run1 = run1Result.value
3438
expect(run1.output.type).not.toEqual('error')
3539
expect(cost1).toBeGreaterThanOrEqual(0)
3640

3741
let cost2 = -1
38-
const run2 = await client.run({
42+
const run2Result = await client.run({
3943
prompt,
4044
agent: 'base',
4145
previousRun: run1,
@@ -46,6 +50,10 @@ describe('Prompt Caching', () => {
4650
},
4751
})
4852

53+
if (!run2Result.success) {
54+
throw new Error(`Run2 failed: ${JSON.stringify(run2Result.error)}`)
55+
}
56+
const run2 = run2Result.value
4957
expect(run2.output.type).not.toEqual('error')
5058
expect(cost2).toBeGreaterThanOrEqual(0)
5159

‎sdk/src/__tests__/validate-agents-network-errors.test.ts‎

Lines changed: 89 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,10 @@ describe('validateAgents network error handling', () => {
2525
test('throws NETWORK_ERROR for connection failures', async () => {
2626
mockFetch.mockRejectedValue(new Error('Failed to fetch'))
2727

28-
await expect(
29-
validateAgents([testAgent], { remote: true })
30-
).rejects.toMatchObject({
31-
code: 'NETWORK_ERROR',
32-
message: expect.stringContaining('Failed to connect to validation API'),
33-
})
28+
const result = await validateAgents([testAgent], { remote: true })
29+
expect(result.success).toBe(false)
30+
expect(result.error.code).toBe('NETWORK_ERROR')
31+
expect(result.error.message).toContain('Failed to connect to validation API')
3432
})
3533

3634
test('throws NETWORK_ERROR for 500 server errors', async () => {
@@ -41,16 +39,12 @@ describe('validateAgents network error handling', () => {
4139
json: () => Promise.resolve({ error: 'Server crashed' }),
4240
} as Response)
4341

44-
try {
45-
await validateAgents([testAgent], { remote: true })
46-
expect(true).toBe(false) // Should not reach
47-
} catch (error: any) {
48-
expect(error.code).toBe('NETWORK_ERROR')
49-
expect(error.message).toContain('Failed to connect')
50-
// The original error with status is wrapped
51-
expect(error.originalError).toBeDefined()
52-
expect(error.originalError.status).toBe(500)
53-
}
42+
const result = await validateAgents([testAgent], { remote: true })
43+
expect(result.success).toBe(false)
44+
expect(result.error.code).toBe('NETWORK_ERROR')
45+
expect(result.error.message).toContain('Failed to connect')
46+
expect(result.error.originalError).toBeDefined()
47+
expect((result.error.originalError as any).status).toBe(500)
5448
})
5549

5650
test('throws NETWORK_ERROR for 502 Bad Gateway', async () => {
@@ -61,15 +55,12 @@ describe('validateAgents network error handling', () => {
6155
json: () => Promise.resolve({}),
6256
} as Response)
6357

64-
try {
65-
await validateAgents([testAgent], { remote: true })
66-
expect(true).toBe(false) // Should not reach
67-
} catch (error: any) {
68-
expect(error.code).toBe('NETWORK_ERROR')
69-
expect(error.message).toContain('Failed to connect')
70-
expect(error.originalError).toBeDefined()
71-
expect(error.originalError.status).toBe(502)
72-
}
58+
const result = await validateAgents([testAgent], { remote: true })
59+
expect(result.success).toBe(false)
60+
expect(result.error.code).toBe('NETWORK_ERROR')
61+
expect(result.error.message).toContain('Failed to connect')
62+
expect(result.error.originalError).toBeDefined()
63+
expect((result.error.originalError as any).status).toBe(502)
7364
})
7465

7566
test('throws NETWORK_ERROR for 503 Service Unavailable', async () => {
@@ -80,15 +71,12 @@ describe('validateAgents network error handling', () => {
8071
json: () => Promise.reject(new Error('Invalid JSON')),
8172
} as Response)
8273

83-
try {
84-
await validateAgents([testAgent], { remote: true })
85-
expect(true).toBe(false) // Should not reach
86-
} catch (error: any) {
87-
expect(error.code).toBe('NETWORK_ERROR')
88-
expect(error.message).toContain('Failed to connect')
89-
expect(error.originalError).toBeDefined()
90-
expect(error.originalError.status).toBe(503)
91-
}
74+
const result = await validateAgents([testAgent], { remote: true })
75+
expect(result.success).toBe(false)
76+
expect(result.error.code).toBe('NETWORK_ERROR')
77+
expect(result.error.message).toContain('Failed to connect')
78+
expect(result.error.originalError).toBeDefined()
79+
expect((result.error.originalError as any).status).toBe(503)
9280
})
9381

9482
test('throws NETWORK_ERROR for 504 Gateway Timeout', async () => {
@@ -99,54 +87,44 @@ describe('validateAgents network error handling', () => {
9987
json: () => Promise.resolve({ error: 'Timeout' }),
10088
} as Response)
10189

102-
try {
103-
await validateAgents([testAgent], { remote: true })
104-
expect(true).toBe(false) // Should not reach
105-
} catch (error: any) {
106-
expect(error.code).toBe('NETWORK_ERROR')
107-
expect(error.message).toContain('Failed to connect')
108-
expect(error.originalError).toBeDefined()
109-
expect(error.originalError.status).toBe(504)
110-
}
90+
const result = await validateAgents([testAgent], { remote: true })
91+
expect(result.success).toBe(false)
92+
expect(result.error.code).toBe('NETWORK_ERROR')
93+
expect(result.error.message).toContain('Failed to connect')
94+
expect(result.error.originalError).toBeDefined()
95+
expect((result.error.originalError as any).status).toBe(504)
11196
})
11297

11398
test('throws NETWORK_ERROR for network timeouts', async () => {
11499
mockFetch.mockRejectedValue(new Error('Network timeout'))
115100

116-
await expect(
117-
validateAgents([testAgent], { remote: true })
118-
).rejects.toMatchObject({
119-
code: 'NETWORK_ERROR',
120-
message: expect.stringContaining('Failed to connect'),
121-
})
101+
const result = await validateAgents([testAgent], { remote: true })
102+
expect(result.success).toBe(false)
103+
expect(result.error.code).toBe('NETWORK_ERROR')
104+
expect(result.error.message).toContain('Failed to connect')
122105
})
123106

124107
test('throws NETWORK_ERROR for DNS resolution failures', async () => {
125108
mockFetch.mockRejectedValue(new Error('getaddrinfo ENOTFOUND api.example.com'))
126109

127-
await expect(
128-
validateAgents([testAgent], { remote: true })
129-
).rejects.toMatchObject({
130-
code: 'NETWORK_ERROR',
131-
message: expect.stringContaining('Failed to connect'),
132-
originalError: expect.objectContaining({
133-
message: expect.stringContaining('ENOTFOUND'),
134-
}),
135-
})
110+
const result = await validateAgents([testAgent], { remote: true })
111+
expect(result.success).toBe(false)
112+
expect(result.error.code).toBe('NETWORK_ERROR')
113+
expect(result.error.message).toContain('Failed to connect')
114+
expect(result.error.originalError).toEqual(
115+
expect.objectContaining({ message: expect.stringContaining('ENOTFOUND') }),
116+
)
136117
})
137118

138119
test('includes original error in network errors', async () => {
139120
const originalError = new Error('Connection refused')
140121
mockFetch.mockRejectedValue(originalError)
141122

142-
try {
143-
await validateAgents([testAgent], { remote: true })
144-
expect(true).toBe(false) // Should not reach here
145-
} catch (error: any) {
146-
expect(error.code).toBe('NETWORK_ERROR')
147-
expect(error.originalError).toBeDefined()
148-
expect(error.originalError.message).toBe('Connection refused')
149-
}
123+
const result = await validateAgents([testAgent], { remote: true })
124+
expect(result.success).toBe(false)
125+
expect(result.error.code).toBe('NETWORK_ERROR')
126+
expect(result.error.originalError).toBeDefined()
127+
expect((result.error.originalError as any).message).toBe('Connection refused')
150128
})
151129
})
152130

@@ -161,10 +139,12 @@ describe('validateAgents network error handling', () => {
161139

162140
const result = await validateAgents([testAgent], { remote: true })
163141

164-
expect(result.success).toBe(false)
165-
expect(result.validationErrors).toHaveLength(1)
166-
expect(result.validationErrors[0].id).toBe('validation_api_error')
167-
expect(result.validationErrors[0].message).toContain('Invalid agent format')
142+
expect(result.success).toBe(true)
143+
const value = result.value
144+
expect(value.success).toBe(false)
145+
expect(value.validationErrors).toHaveLength(1)
146+
expect(value.validationErrors[0].id).toBe('validation_api_error')
147+
expect(value.validationErrors[0].message).toContain('Invalid agent format')
168148
})
169149

170150
test('returns validation error for 401 Unauthorized', async () => {
@@ -177,9 +157,11 @@ describe('validateAgents network error handling', () => {
177157

178158
const result = await validateAgents([testAgent], { remote: true })
179159

180-
expect(result.success).toBe(false)
181-
expect(result.validationErrors).toHaveLength(1)
182-
expect(result.validationErrors[0].id).toBe('validation_api_error')
160+
expect(result.success).toBe(true)
161+
const value = result.value
162+
expect(value.success).toBe(false)
163+
expect(value.validationErrors).toHaveLength(1)
164+
expect(value.validationErrors[0].id).toBe('validation_api_error')
183165
})
184166

185167
test('returns validation error for 403 Forbidden', async () => {
@@ -192,9 +174,11 @@ describe('validateAgents network error handling', () => {
192174

193175
const result = await validateAgents([testAgent], { remote: true })
194176

195-
expect(result.success).toBe(false)
196-
expect(result.validationErrors[0].id).toBe('validation_api_error')
197-
expect(result.validationErrors[0].message).toContain('Access denied')
177+
expect(result.success).toBe(true)
178+
const value = result.value
179+
expect(value.success).toBe(false)
180+
expect(value.validationErrors[0].id).toBe('validation_api_error')
181+
expect(value.validationErrors[0].message).toContain('Access denied')
198182
})
199183

200184
test('returns validation error for 404 Not Found', async () => {
@@ -207,8 +191,10 @@ describe('validateAgents network error handling', () => {
207191

208192
const result = await validateAgents([testAgent], { remote: true })
209193

210-
expect(result.success).toBe(false)
211-
expect(result.validationErrors[0].message).toContain('404')
194+
expect(result.success).toBe(true)
195+
const value = result.value
196+
expect(value.success).toBe(false)
197+
expect(value.validationErrors[0].message).toContain('404')
212198
})
213199

214200
test('returns validation error for 422 Unprocessable Entity', async () => {
@@ -221,9 +207,11 @@ describe('validateAgents network error handling', () => {
221207

222208
const result = await validateAgents([testAgent], { remote: true })
223209

224-
expect(result.success).toBe(false)
225-
expect(result.validationErrors[0].id).toBe('validation_api_error')
226-
expect(result.validationErrors[0].message).toContain('Validation failed')
210+
expect(result.success).toBe(true)
211+
const value = result.value
212+
expect(value.success).toBe(false)
213+
expect(value.validationErrors[0].id).toBe('validation_api_error')
214+
expect(value.validationErrors[0].message).toContain('Validation failed')
227215
})
228216

229217
test('handles JSON parse errors in 4xx responses gracefully', async () => {
@@ -236,8 +224,10 @@ describe('validateAgents network error handling', () => {
236224

237225
const result = await validateAgents([testAgent], { remote: true })
238226

239-
expect(result.success).toBe(false)
240-
expect(result.validationErrors[0].message).toContain('400')
227+
expect(result.success).toBe(true)
228+
const value = result.value
229+
expect(value.success).toBe(false)
230+
expect(value.validationErrors[0].message).toContain('400')
241231
})
242232
})
243233

@@ -251,8 +241,10 @@ describe('validateAgents network error handling', () => {
251241
const result = await validateAgents([testAgent], { remote: true })
252242

253243
expect(result.success).toBe(true)
254-
expect(result.validationErrors).toEqual([])
255-
expect(result.errorCount).toBe(0)
244+
const value = result.value
245+
expect(value.success).toBe(true)
246+
expect(value.validationErrors).toEqual([])
247+
expect(value.errorCount).toBe(0)
256248
})
257249

258250
test('returns validation errors from successful API response', async () => {
@@ -268,10 +260,12 @@ describe('validateAgents network error handling', () => {
268260

269261
const result = await validateAgents([testAgent], { remote: true })
270262

271-
expect(result.success).toBe(false)
272-
expect(result.validationErrors).toHaveLength(2)
273-
expect(result.validationErrors[0].id).toBe('agent1.yaml')
274-
expect(result.validationErrors[0].message).toBe('Missing required field')
263+
expect(result.success).toBe(true)
264+
const value = result.value
265+
expect(value.success).toBe(false)
266+
expect(value.validationErrors).toHaveLength(2)
267+
expect(value.validationErrors[0].id).toBe('agent1.yaml')
268+
expect(value.validationErrors[0].message).toBe('Missing required field')
275269
})
276270
})
277271

@@ -282,8 +276,10 @@ describe('validateAgents network error handling', () => {
282276

283277
expect(mockFetch).not.toHaveBeenCalled()
284278
// Local validation should work without network
285-
expect(result).toHaveProperty('success')
286-
expect(result).toHaveProperty('validationErrors')
279+
expect(result.success).toBe(true)
280+
const value = result.value
281+
expect(value).toHaveProperty('success')
282+
expect(value).toHaveProperty('validationErrors')
287283
})
288284

289285
test('local validation is not affected by network issues', async () => {
@@ -293,7 +289,8 @@ describe('validateAgents network error handling', () => {
293289
const result = await validateAgents([testAgent], { remote: false })
294290

295291
expect(mockFetch).not.toHaveBeenCalled()
296-
expect(result).toHaveProperty('success')
292+
expect(result.success).toBe(true)
293+
expect(result.value).toHaveProperty('success')
297294
})
298295
})
299296
})

‎sdk/src/client.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
1-
import { BACKEND_URL, WEBSITE_URL } from './constants'
1+
import { WEBSITE_URL } from './constants'
22
import { run } from './run'
33
import { API_KEY_ENV_VAR } from '../../common/src/old-constants'
44

5-
import type { RunOptions, CodebuffClientOptions } from './run'
6-
import type { RunState } from './run-state'
5+
import type { RunOptions, CodebuffClientOptions, RunResult } from './run'
76

87
export class CodebuffClient {
98
public options: CodebuffClientOptions & {
@@ -65,7 +64,7 @@ export class CodebuffClient {
6564
*/
6665
public async run(
6766
options: RunOptions & CodebuffClientOptions,
68-
): Promise<RunState> {
67+
): Promise<RunResult> {
6968
return run({ ...this.options, ...options })
7069
}
7170

0 commit comments

Comments
 (0)