- 
                Notifications
    You must be signed in to change notification settings 
- Fork 771
fix: handle error from bedrock batch status check #1369
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
base: main
Are you sure you want to change the base?
Conversation
| Description🔄 What Changed
 🔍 Impact of the Change
 📁 Total Files Changed
 🧪 Test Added
 🔒Security Vulnerabilities
 Motivation
 Type of Change
 How Has This Been Tested?
 Screenshots (if applicable)N/A Checklist
 Related IssuesN/A Tip Quality Recommendations
 Tanka Poem ♫
 Sequence DiagramsequenceDiagram
    participant Client
    participant Gateway as Gateway (getBatchOutput)
    participant BedrockAPI as Bedrock API
    Note over Gateway: getModelProvider(modelId) updated to include 'amazon' -> 'titan'
    Client->>Gateway: requestBatchOutput(batchId, modelId)
    Gateway->>BedrockAPI: GET /batch/status (batchId)
    alt Bedrock API Response is NOT OK
        BedrockAPI-->>Gateway: Error Response (status, errorText)
        Gateway->>Gateway: Parse errorText into structured error
        Gateway-->>Client: Error Response (status, {error, provider: BEDROCK})
    else Bedrock API Response is OK
        BedrockAPI-->>Gateway: Success Response (batchDetails)
        Gateway->>Gateway: Extract outputFileId from batchDetails
        Gateway-->>Client: Batch Output (outputFileId)
    end
 | 
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.
Error handling added for bedrock batch status check. Minor improvements suggested for error structure and provider mapping.
| const error = await retrieveBatchesResponse.text(); | ||
| const _error = { | ||
| message: error, | ||
| param: null, | ||
| type: null, | ||
| }; | 
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.
🔴 Security/Data Exposure
Issue: Error response includes raw error text which may expose internal details
Fix: Sanitize error message or use a generic message for production
Impact: Prevents leaking internal system details to clients
| const error = await retrieveBatchesResponse.text(); | |
| const _error = { | |
| message: error, | |
| param: null, | |
| type: null, | |
| }; | |
| const error = await retrieveBatchesResponse.text(); | |
| const _error = { | |
| message: 'Batch retrieval failed', | |
| param: null, | |
| type: null, | |
| }; | 
| type: null, | ||
| }; | ||
| return new Response( | ||
| JSON.stringify({ error: _error, provider: BEDROCK }), | 
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.
please follow the exisitng format for provider errors
Bedrock error: {errorMessage}
you can use the generateErrorResponse() utility present in /providers/utils
Description
Handle error from bedrock and return the error for batch output
Motivation
Type of Change
How Has This Been Tested?
Screenshots (if applicable)
Checklist
Related Issues