Handle non-200 status codes from Claude API #36
Conversation
WalkthroughAdded an HTTP status code check in src/claude/claude.go to return an error for non-200 responses before decoding the body, altering control flow and error handling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Caller
participant CL as ClaudeClient
participant H as HTTP API
C->>CL: RequestCompletion(input)
CL->>H: POST /complete
H-->>CL: HTTP Response (status, body)
alt Status != 200
note right of CL: New behavior
CL-->>C: error("claude AI response <code>")
else Status == 200
CL->>CL: Decode body
CL-->>C: Completion result
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![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
🧹 Nitpick comments (1)
src/claude/claude.go (1)
70-72: Consider adding tests for error handling.Since this PR introduces new error handling logic, it would be beneficial to add automated tests covering:
- Non-200 status codes (e.g., 400, 401, 429, 500)
- Verification that error messages include status codes and response details
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/claude/claude.go(1 hunks)
🔇 Additional comments (1)
src/claude/claude.go (1)
70-72: Other AI provider implementations handle errors correctly
Grok already reads and includes the response body on non-200 status, and Gemini uses thegenaiclient (which surfaces HTTP errors internally), so no changes are needed for those providers.
| if resp.StatusCode != http.StatusOK { | ||
| return "", fmt.Errorf("claude AI response %d", resp.StatusCode) | ||
| } |
There was a problem hiding this comment.
Read and include the error response body for better diagnostics.
The error message lacks details from the Claude API's response body. When the API returns a non-200 status, it typically includes a JSON error response with valuable information (e.g., rate limit details, authentication failures, invalid request specifics). Discarding this information makes debugging significantly harder for users.
Additionally, the error message "claude AI response %d" could be clearer.
Apply this diff to read the response body and include error details:
if resp.StatusCode != http.StatusOK {
- return "", fmt.Errorf("claude AI response %d", resp.StatusCode)
+ body, _ := io.ReadAll(resp.Body)
+ return "", fmt.Errorf("claude API request failed with status code %d: %s", resp.StatusCode, string(body))
}Don't forget to add the io import:
import (
"bytes"
"context"
"encoding/json"
"fmt"
+ "io"
"net/http"
"github.com/dfanso/commit-msg/src/types"
)🤖 Prompt for AI Agents
In src/claude/claude.go around lines 70 to 72, the code only returns a generic
error for non-200 responses; read the response body (remember to add the io
import) and include its contents in the returned error for better diagnostics.
Specifically, defer resp.Body.Close(), use io.ReadAll(resp.Body) (handle that
read error), trim or convert the body to a string, and return a clearer error
such as "claude AI error: status %d: %s" including both resp.StatusCode and the
response body content.
DFanso
added
bug
There was a problem hiding this comment.
hey @GauravJangra9988 can u check coderabbitai review also
DFanso
added
good first issue
|
Checked the Code Rabbit review it is OK. |
2 participants
Description
Handle the status other then OK , for claude API request.
Type of Change
Related Issue
#20
Fixes #(issue number)
#20
Changes Made
Testing
Checklist
Screenshots (if applicable)
Additional Notes
For Hacktoberfest Participants
Thank you for your contribution! 🎉
Summary by CodeRabbit