fix: handle provider error logs addition #1241
Conversation
|
Summary By MatterAI
🔄 What ChangedThis PR improves error handling in the gateway by enhancing error logging and response formatting when provider errors occur. The changes focus on better error reporting and standardizing error responses. 🔍 Impact of the ChangeThe changes improve error visibility and debugging by adding detailed error logging (message, cause, stack) when errors occur in the 📁 Total Files Changed
🧪 Test AddedN/A - No new tests were added in this PR. 🔒 Security VulnerabilitiesN/A - No security vulnerabilities were identified in this PR. Tip Quality Recommendations
Sequence DiagramsequenceDiagram
participant Client
participant Gateway
participant tryTargetsRecursively
participant Provider
Client->>Gateway: Request
Gateway->>tryTargetsRecursively: Process request
tryTargetsRecursively->>Provider: Forward request
alt Provider Success
Provider-->>tryTargetsRecursively: Success Response
tryTargetsRecursively-->>Gateway: Return provider response
else Provider Error
Provider-->>tryTargetsRecursively: Error
Note over tryTargetsRecursively: Enhanced Error Logging:
Note over tryTargetsRecursively: console.error(error.message, error.cause, error.stack)
alt GatewayError
tryTargetsRecursively-->>Gateway: Response with error.message
else Other Error
tryTargetsRecursively-->>Gateway: Response with "Something went wrong"
end
Note over tryTargetsRecursively: Set status: 500
Note over tryTargetsRecursively: Set header: x-portkey-gateway-exception: true
end
Gateway-->>Client: Final Response
|
![matter-code-review[bot]](https://avatars.githubusercontent.com/in/474549?s=60&v=4){kind=small}
|
|
||
| this.updateHeaders(finalMappedResponse, cache.cacheStatus, retryAttempt); | ||
|
|
||
| if (!finalMappedResponse.ok) { |
There was a problem hiding this comment.
Instead of throwing error, tryPost will return it just as a simple response as throwing error is redundant.
In tryTargetsRecursively, we are already checking for response.ok
| }, | ||
| } | ||
| ); | ||
| } else { |
There was a problem hiding this comment.
Since this PR removes the throw new Error() logic in tryPost for LLM failures, we no longer need to set it here. If the any error is caught, they will always be an Gateway exception.
|
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
|
2 participants
Description
logObject.log() method is not getting called for error responses because responseService.create() throws an error with response details for all non-2xx LLM error responses. This causes issues with keeping track of logObjects of failed requests.
It is redundant to throw the error from responseService, because it is simply getting caught and then the same value is used in the final response object of tryTargetsRecursively function.
With this change, tryTargetsRecursively catch block should only be used when there is an actual gateway exception. For all provider non-2xx responses, they will be treated as a valid response.
Motivation
Fix on top of #1121
Type of Change
How Has This Been Tested?
Screenshots (if applicable)
Checklist
Related Issues