fix: improve error handling for AI service responses #49
Conversation
- Add defensive check for response.body existence to prevent undefined property access - Include x-ms-error-code header in error messages for better debugging - Provide clearer error messages for different failure scenarios - Fix 'Cannot read properties of undefined (reading 'error')' runtime error This improves the debugging experience when AI service requests fail due to network issues, authentication errors, or unexpected response formats.
There was a problem hiding this comment.
Pull Request Overview
Improves robustness of AI service response handling by extracting header error codes and preventing undefined body access.
- Extract
x-ms-error-codefrom response headers for richer error context - Guard against missing
response.bodybefore property access - Enhance thrown error messages with status, error code, and serialized body
Comments suppressed due to low confidence (1)
src/main.ts:95
- This branch handles missing
response.body—add unit tests to verify that the correct error is thrown when the body is undefined.
if (!response.body) {
| @@ -82,14 +82,27 @@ export async function run(): Promise<void> { | |||
| }) | |||
|
|
|||
| if (isUnexpected(response)) { | |||
| if (response.body.error) { | |||
| // Extract x-ms-error-code from headers if available | |||
| const errorCode = response.headers['x-ms-error-code'] | |||
There was a problem hiding this comment.
Depending on your HTTP client, headers may need to be accessed via a get method (e.g., response.headers.get('x-ms-error-code')) rather than direct property indexing.
| const errorCode = response.headers['x-ms-error-code'] | |
| const errorCode = response.headers.get('x-ms-error-code') |
Copilot uses AI. Check for mistakes.
| const errorCodeMsg = errorCode ? ` (error code: ${errorCode})` : '' | ||
|
|
||
| // Check if response body exists and contains error details | ||
| if (response.body && response.body.error) { | ||
| throw response.body.error |
There was a problem hiding this comment.
[nitpick] Throwing response.body.error directly may not yield an Error instance or include the errorCodeMsg. Consider wrapping it in an Error, for example: throw new Error(${response.body.error.message}${errorCodeMsg}).
| throw response.body.error | |
| throw new Error(`${response.body.error.message}${errorCodeMsg}`) |
Copilot uses AI. Check for mistakes.
|
@sgoedecke Thanks for the review! I’ve fixed the linting issues and all unit tests are now passing. Unfortunately, the integration tests are still failing: The improved error message you see above comes from this PR, so the failure itself may be caused by something outside these changes. Could you take a look when you have a chance? |
|
Yeah, you're right - the errors are because Models isn't enabled for the |
Improve error handling to prevent undefined property access errors and add x-ms-error-code header information for better debugging.