translator: handling for OpenAI and AWSBedrock error response #152
Conversation
yuzisun
changed the title
|
can you fix the e2e |
| if v, ok := headers[statusHeaderName]; ok && v != string(typev3.StatusCode_OK) { | ||
| return nil, nil | ||
| } |
There was a problem hiding this comment.
I think this is not necessary or should be panic... if we reach this, that means a bug in extproc code
There was a problem hiding this comment.
Actually for header processing we need this check otherwise it continues processing the event stream header mutation.
extprocapi/extproc.go
Outdated
|
so i made it easy to add the end-to-end transformation testing with testupstream here: feel free to add more cases to cover this case (maybe some fix might be necessary for the test as well as testupstream) |
Signed-off-by: Dan Sun <[email protected]>
Signed-off-by: Dan Sun <[email protected]>
Signed-off-by: Dan Sun <[email protected]>
yuzisun
force-pushed
the
rename
branch
2 times, most recently
from
425230d to
7acccb4
Compare
Signed-off-by: Dan Sun <[email protected]>
Signed-off-by: Dan Sun <[email protected]>
Signed-off-by: Dan Sun <[email protected]>
yuzisun
force-pushed
the
rename
branch
4 times, most recently
from
675ee46 to
8b1c09d
Compare
Signed-off-by: Dan Sun <[email protected]>
| if v, err := strconv.Atoi(v); err == nil { | ||
| if !isGoodStatusCode(v) { | ||
| return nil, nil | ||
| } | ||
| } |
There was a problem hiding this comment.
also i am still not sure why if this is a bad status code we can continue and when good we return early. could you put the detailed code comments. This will be hard to understand by anyone without the extensive knowledge. I believe the comments will be helpful for future contributors (including me!)
There was a problem hiding this comment.
I rewrote the code in a way that it does not require this logic. Before it incorrectly returns a content type check error on error response. I am not a fan of writing too much code comments which means the code is not self-explainable.
Signed-off-by: Dan Sun <[email protected]>
Signed-off-by: Dan Sun <[email protected]>
Added the tests all ready to go |
|
Will take a look with an hour! |
Add handling for the OpenAI and Bedrock error response. Currently when error response is returned it fails on decoding the
ConverseOutputorOpenAIChatCompletionResponseas it is assuming a 200 response.For AWSBedrock the error response is converted to OpenAIError type when returning to the user. For OpenAI error response we return directly back without translation. In case of connection errors to the AWS Bedrock or OpenAI backend, we translate to OpenAIError type based on the status code and message body.
fixes #133