include deepgram specific error message in StatusError.Error #267
Conversation
WalkthroughThe changes involve an update to the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant Deepgram
Client->>Server: Sends HTTP request
Server->>Deepgram: Forwards request
Deepgram-->>Server: Returns response with error
Server->>Server: Checks for DeepgramError
alt DeepgramError exists
Server->>Server: Format error message with details
else
Server->>Server: Use default error message format
end
Server-->>Client: Returns error message
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Hi @cbodonnell
I can't believe that wasn't there, but thanks for catching that and also opening the PR for it! I appreciate it!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- pkg/client/interfaces/v1/utils.go (1 hunks)
🔇 Additional comments (2)
pkg/client/interfaces/v1/utils.go (2)
98-100: LGTM! The implementation effectively enhances error reporting.
The changes successfully fulfill the PR objective by incorporating Deepgram-specific error messages when available, while maintaining backward compatibility through the fallback case.
98-100: Add unit tests for the enhanced error formatting.
The changes would benefit from unit tests to verify the error message formatting in different scenarios:
- With DeepgramError containing only ErrMsg
- With DeepgramError containing all fields
- With nil DeepgramError (fallback case)
Would you like me to help generate comprehensive unit tests for these scenarios?
Proposed changes
This PR adds the additional Deepgram-specific error message to the
StatusError.Errorfunction, if present. The goal behind this change is to provide additional details to consumers of the SDK (for example the reason why a 400 error was returned). For example, the following error does not explain what was wrong with the request:With the changes in this PR, the above would be returned as something like:
Types of changes
What types of changes does your code introduce to the community Go SDK?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
It's possible to check the returned error type and get these details within the calling code, but ideally the error message itself contains enough details to know what went wrong. Open to thoughts from the team if there are better options here.
Summary by CodeRabbit
StatusErrorto provide more detailed information when aDeepgramErroris present.