Skip to content

Commit 6401238

Browse files
cutecatfanncutecatfann
authored
Check HTTP status codes in RawClient Delete() and Post()
- Added checks for Oauth bug - Added corresponding unit tests
2 parents 14d2d4e + e965a5f commit 6401238

2 files changed

Lines changed: 265 additions & 7 deletions

File tree

‎pkg/desktop/raw_client.go‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -134,11 +134,6 @@ func (c *RawClient) Post(ctx context.Context, endpoint string, v any, result any
134134
}
135135
defer response.Body.Close()
136136

137-
if result == nil {
138-
_, err := io.Copy(io.Discard, response.Body)
139-
return err
140-
}
141-
142137
buf, err := io.ReadAll(response.Body)
143138
if err != nil {
144139
return err
@@ -156,6 +151,10 @@ func (c *RawClient) Post(ctx context.Context, endpoint string, v any, result any
156151
return fmt.Errorf("HTTP %d: %s", response.StatusCode, string(buf))
157152
}
158153

154+
if result == nil {
155+
return nil
156+
}
157+
159158
if err := json.Unmarshal(buf, &result); err != nil {
160159
return err
161160
}
@@ -179,6 +178,22 @@ func (c *RawClient) Delete(ctx context.Context, endpoint string) error {
179178
}
180179
defer response.Body.Close()
181180

182-
_, err = io.Copy(io.Discard, response.Body)
183-
return err
181+
buf, err := io.ReadAll(response.Body)
182+
if err != nil {
183+
return err
184+
}
185+
186+
// Check HTTP status code - return error for non-2xx responses
187+
if response.StatusCode < 200 || response.StatusCode >= 300 {
188+
// Try to parse error message from response
189+
var errorMsg struct {
190+
Message string `json:"message"`
191+
}
192+
if json.Unmarshal(buf, &errorMsg) == nil && errorMsg.Message != "" {
193+
return fmt.Errorf("HTTP %d: %s", response.StatusCode, errorMsg.Message)
194+
}
195+
return fmt.Errorf("HTTP %d: %s", response.StatusCode, string(buf))
196+
}
197+
198+
return nil
184199
}

‎pkg/desktop/raw_client_test.go‎

Lines changed: 243 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,243 @@
1+
package desktop
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"net"
7+
"net/http"
8+
"net/http/httptest"
9+
"testing"
10+
)
11+
12+
// newTestClient creates a RawClient that connects to an httptest.Server
13+
func newTestClient(t *testing.T, handler http.Handler) *RawClient {
14+
t.Helper()
15+
server := httptest.NewServer(handler)
16+
t.Cleanup(server.Close)
17+
18+
client := &RawClient{
19+
client: func() *http.Client {
20+
return &http.Client{
21+
Transport: &http.Transport{
22+
DialContext: func(_ context.Context, _, _ string) (net.Conn, error) {
23+
var d net.Dialer
24+
return d.DialContext(context.Background(), "tcp", server.Listener.Addr().String())
25+
},
26+
},
27+
}
28+
},
29+
timeout: 10 * 1e9, // 10s
30+
}
31+
return client
32+
}
33+
34+
func writeJSON(t *testing.T, w http.ResponseWriter, v any) {
35+
t.Helper()
36+
if err := json.NewEncoder(w).Encode(v); err != nil {
37+
t.Fatalf("failed to encode JSON response: %v", err)
38+
}
39+
}
40+
41+
func TestGet_Success(t *testing.T) {
42+
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
43+
if r.Method != http.MethodGet {
44+
t.Errorf("expected GET, got %s", r.Method)
45+
}
46+
w.WriteHeader(http.StatusOK)
47+
writeJSON(t, w, map[string]string{"key": "value"})
48+
})
49+
50+
client := newTestClient(t, handler)
51+
52+
var result map[string]string
53+
err := client.Get(context.Background(), "/test", &result)
54+
if err != nil {
55+
t.Fatalf("unexpected error: %v", err)
56+
}
57+
if result["key"] != "value" {
58+
t.Errorf("expected key=value, got key=%s", result["key"])
59+
}
60+
}
61+
62+
func TestGet_ErrorStatus(t *testing.T) {
63+
handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
64+
w.WriteHeader(http.StatusInternalServerError)
65+
writeJSON(t, w, map[string]string{"message": "something broke"})
66+
})
67+
68+
client := newTestClient(t, handler)
69+
70+
var result map[string]string
71+
err := client.Get(context.Background(), "/test", &result)
72+
if err == nil {
73+
t.Fatal("expected error, got nil")
74+
}
75+
expected := "HTTP 500: something broke"
76+
if err.Error() != expected {
77+
t.Errorf("expected %q, got %q", expected, err.Error())
78+
}
79+
}
80+
81+
func TestGet_ErrorStatusPlainBody(t *testing.T) {
82+
handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
83+
w.WriteHeader(http.StatusNotFound)
84+
if _, err := w.Write([]byte("404 Not Found")); err != nil {
85+
t.Fatalf("failed to write response: %v", err)
86+
}
87+
})
88+
89+
client := newTestClient(t, handler)
90+
91+
var result map[string]string
92+
err := client.Get(context.Background(), "/test", &result)
93+
if err == nil {
94+
t.Fatal("expected error, got nil")
95+
}
96+
expected := "HTTP 404: 404 Not Found"
97+
if err.Error() != expected {
98+
t.Errorf("expected %q, got %q", expected, err.Error())
99+
}
100+
}
101+
102+
func TestPost_Success(t *testing.T) {
103+
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
104+
if r.Method != http.MethodPost {
105+
t.Errorf("expected POST, got %s", r.Method)
106+
}
107+
w.WriteHeader(http.StatusOK)
108+
writeJSON(t, w, map[string]string{"status": "ok"})
109+
})
110+
111+
client := newTestClient(t, handler)
112+
113+
var result map[string]string
114+
err := client.Post(context.Background(), "/test", map[string]string{"input": "data"}, &result)
115+
if err != nil {
116+
t.Fatalf("unexpected error: %v", err)
117+
}
118+
if result["status"] != "ok" {
119+
t.Errorf("expected status=ok, got status=%s", result["status"])
120+
}
121+
}
122+
123+
func TestPost_ErrorStatus(t *testing.T) {
124+
handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
125+
w.WriteHeader(http.StatusBadRequest)
126+
writeJSON(t, w, map[string]string{"message": "bad input"})
127+
})
128+
129+
client := newTestClient(t, handler)
130+
131+
var result map[string]string
132+
err := client.Post(context.Background(), "/test", map[string]string{"input": "data"}, &result)
133+
if err == nil {
134+
t.Fatal("expected error, got nil")
135+
}
136+
expected := "HTTP 400: bad input"
137+
if err.Error() != expected {
138+
t.Errorf("expected %q, got %q", expected, err.Error())
139+
}
140+
}
141+
142+
func TestPost_NilResult_Success(t *testing.T) {
143+
handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
144+
w.WriteHeader(http.StatusOK)
145+
})
146+
147+
client := newTestClient(t, handler)
148+
149+
err := client.Post(context.Background(), "/test", nil, nil)
150+
if err != nil {
151+
t.Fatalf("unexpected error: %v", err)
152+
}
153+
}
154+
155+
func TestPost_NilResult_ErrorStatus(t *testing.T) {
156+
handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
157+
w.WriteHeader(http.StatusInternalServerError)
158+
writeJSON(t, w, map[string]string{"message": "server error"})
159+
})
160+
161+
client := newTestClient(t, handler)
162+
163+
err := client.Post(context.Background(), "/test", nil, nil)
164+
if err == nil {
165+
t.Fatal("expected error, got nil")
166+
}
167+
expected := "HTTP 500: server error"
168+
if err.Error() != expected {
169+
t.Errorf("expected %q, got %q", expected, err.Error())
170+
}
171+
}
172+
173+
func TestDelete_Success(t *testing.T) {
174+
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
175+
if r.Method != http.MethodDelete {
176+
t.Errorf("expected DELETE, got %s", r.Method)
177+
}
178+
if r.URL.Path != "/apps/test-app" {
179+
t.Errorf("expected path /apps/test-app, got %s", r.URL.Path)
180+
}
181+
w.WriteHeader(http.StatusOK)
182+
})
183+
184+
client := newTestClient(t, handler)
185+
186+
err := client.Delete(context.Background(), "/apps/test-app")
187+
if err != nil {
188+
t.Fatalf("unexpected error: %v", err)
189+
}
190+
}
191+
192+
func TestDelete_204NoContent(t *testing.T) {
193+
handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
194+
w.WriteHeader(http.StatusNoContent)
195+
})
196+
197+
client := newTestClient(t, handler)
198+
199+
err := client.Delete(context.Background(), "/apps/test-app")
200+
if err != nil {
201+
t.Fatalf("unexpected error: %v", err)
202+
}
203+
}
204+
205+
func TestDelete_ErrorStatusJSON(t *testing.T) {
206+
handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
207+
w.WriteHeader(http.StatusInternalServerError)
208+
writeJSON(t, w, map[string]string{
209+
"message": "provider `cloudflare-autorag`: could not revoke token",
210+
})
211+
})
212+
213+
client := newTestClient(t, handler)
214+
215+
err := client.Delete(context.Background(), "/apps/cloudflare-autorag")
216+
if err == nil {
217+
t.Fatal("expected error, got nil")
218+
}
219+
expected := "HTTP 500: provider `cloudflare-autorag`: could not revoke token"
220+
if err.Error() != expected {
221+
t.Errorf("expected %q, got %q", expected, err.Error())
222+
}
223+
}
224+
225+
func TestDelete_ErrorStatusPlainBody(t *testing.T) {
226+
handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
227+
w.WriteHeader(http.StatusNotFound)
228+
if _, err := w.Write([]byte("404 Not Found")); err != nil {
229+
t.Fatalf("failed to write response: %v", err)
230+
}
231+
})
232+
233+
client := newTestClient(t, handler)
234+
235+
err := client.Delete(context.Background(), "/apps/nonexistent")
236+
if err == nil {
237+
t.Fatal("expected error, got nil")
238+
}
239+
expected := "HTTP 404: 404 Not Found"
240+
if err.Error() != expected {
241+
t.Errorf("expected %q, got %q", expected, err.Error())
242+
}
243+
}

0 commit comments

Comments
 (0)