-
Notifications
You must be signed in to change notification settings - Fork 760
fix: transform x-ai error response to open-ai format #1250
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR adds error response transformation for X-AI to match the OpenAI format, which is a good improvement for consistency. I've identified a few improvements that could make the error handling more robust.
let _response = response as XAIErrorResponse; | ||
if ('error' in _response) { | ||
return { | ||
error: { | ||
message: _response.error as string, | ||
code: _response.code ?? null, | ||
param: null, | ||
type: null, | ||
}, | ||
provider: X_AI, | ||
}; | ||
} | ||
return response; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐛 Bug Fix
Issue: The error handling doesn't properly handle the case when the error property is an object rather than a string, despite the interface defining it as potentially being an object.
Fix: Add proper type checking and handle both string and object error formats.
Impact: Prevents potential runtime errors when X-AI returns an error object instead of a string.
let _response = response as XAIErrorResponse; | |
if ('error' in _response) { | |
return { | |
error: { | |
message: _response.error as string, | |
code: _response.code ?? null, | |
param: null, | |
type: null, | |
}, | |
provider: X_AI, | |
}; | |
} | |
return response; | |
let _response = response as XAIErrorResponse; | |
if ('error' in _response) { | |
return { | |
error: { | |
message: typeof _response.error === 'string' | |
? _response.error | |
: _response.error?.message ?? 'Unknown error', | |
code: _response.code ?? _response.error?.code ?? null, | |
param: _response.error?.param ?? null, | |
type: _response.error?.type ?? null, | |
}, | |
provider: X_AI, | |
}; | |
} | |
return response; |
code?: string; | ||
} | ||
|
||
const xAIResponseTransform = <T>(response: T) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Code Refactor
Issue: The function lacks documentation explaining its purpose and behavior.
Fix: Add JSDoc comments to explain the function's purpose and return value.
Impact: Improves code maintainability and makes it easier for other developers to understand the transformation logic.
const xAIResponseTransform = <T>(response: T) => { | |
/** | |
* Transforms X-AI error responses to match the OpenAI error format | |
* for consistent error handling across providers. | |
* @param response The original response from X-AI | |
* @returns Transformed response in OpenAI-compatible format or the original response if not an error | |
*/ | |
const xAIResponseTransform = <T>(response: T) => { |
Important PR Review SkippedPR review skipped as per the configuration setting. Run a manually review by commenting /matter review 💡Tips to use Matter AICommand List
|
Description
Transfrom X-AI Error responses to OAI format.
Motivation
Type of Change
How Has This Been Tested?
Screenshots (if applicable)
Checklist
Related Issues