Skip to content

Commit 06521b7

Browse files
author
Baton Admin
committed
chore: update connector skills via baton-admin
1 parent c8e2f4e commit 06521b7

File tree

1 file changed

+184
-0
lines changed

1 file changed

+184
-0
lines changed
Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
# patterns-http-safety
2+
3+
HTTP response handling and nil pointer safety.
4+
5+
---
6+
7+
## The Problem
8+
9+
HTTP errors can leave `resp` as nil. Accessing `resp.Body` or `resp.StatusCode` in error paths causes panics.
10+
11+
This is the #3 bug pattern: 13 panic fixes across 12+ repos.
12+
13+
---
14+
15+
## Wrong Pattern
16+
17+
```go
18+
resp, err := client.Do(req)
19+
if err != nil {
20+
// PANIC: resp is nil on network errors
21+
log.Printf("Error: %v, Status: %d", err, resp.StatusCode)
22+
return err
23+
}
24+
defer resp.Body.Close()
25+
```
26+
27+
---
28+
29+
## Correct Pattern
30+
31+
```go
32+
resp, err := client.Do(req)
33+
if err != nil {
34+
// resp MAY be nil - check before using
35+
if resp != nil {
36+
defer resp.Body.Close()
37+
body, _ := io.ReadAll(resp.Body)
38+
return fmt.Errorf("request failed (status %d): %s: %w", resp.StatusCode, body, err)
39+
}
40+
return fmt.Errorf("request failed: %w", err)
41+
}
42+
defer resp.Body.Close()
43+
```
44+
45+
---
46+
47+
## Defer Placement
48+
49+
**Wrong - defer before error check:**
50+
```go
51+
resp, err := client.Do(req)
52+
defer resp.Body.Close() // PANIC if resp is nil
53+
if err != nil {
54+
return err
55+
}
56+
```
57+
58+
**Correct - defer after error check:**
59+
```go
60+
resp, err := client.Do(req)
61+
if err != nil {
62+
return err
63+
}
64+
defer resp.Body.Close()
65+
```
66+
67+
---
68+
69+
## Map Type Assertions
70+
71+
**Wrong - direct assertion panics on missing key:**
72+
```go
73+
userID := data["user_id"].(string) // PANIC if missing or wrong type
74+
```
75+
76+
**Correct - two-value form:**
77+
```go
78+
userID, ok := data["user_id"].(string)
79+
if !ok {
80+
return fmt.Errorf("user_id missing or not string")
81+
}
82+
```
83+
84+
---
85+
86+
## ParentResourceId Access
87+
88+
**Wrong - direct access without nil check:**
89+
```go
90+
parentID := resource.ParentResourceId.Resource // PANIC if nil
91+
```
92+
93+
**Correct - nil check first:**
94+
```go
95+
var parentID string
96+
if resource.ParentResourceId != nil {
97+
parentID = resource.ParentResourceId.Resource
98+
}
99+
```
100+
101+
---
102+
103+
## Error Check Ordering
104+
105+
Always check error before using returned values:
106+
107+
```go
108+
// WRONG
109+
result, err := doSomething()
110+
fmt.Println(result.Value) // Use before check
111+
if err != nil {
112+
return err
113+
}
114+
115+
// CORRECT
116+
result, err := doSomething()
117+
if err != nil {
118+
return err
119+
}
120+
fmt.Println(result.Value) // Use after check
121+
```
122+
123+
---
124+
125+
## HTTP Status Handling
126+
127+
```go
128+
func handleResponse(resp *http.Response) error {
129+
switch resp.StatusCode {
130+
case http.StatusOK, http.StatusCreated, http.StatusNoContent:
131+
return nil
132+
case http.StatusNotFound:
133+
return nil // Often not an error - resource doesn't exist
134+
case http.StatusUnauthorized:
135+
return fmt.Errorf("baton-myservice: unauthorized (check credentials)")
136+
case http.StatusForbidden:
137+
return fmt.Errorf("baton-myservice: forbidden (check permissions)")
138+
case http.StatusTooManyRequests:
139+
return fmt.Errorf("baton-myservice: rate limited") // SDK retries
140+
default:
141+
if resp.StatusCode >= 500 {
142+
return fmt.Errorf("baton-myservice: server error %d", resp.StatusCode)
143+
}
144+
return fmt.Errorf("baton-myservice: unexpected status %d", resp.StatusCode)
145+
}
146+
}
147+
```
148+
149+
---
150+
151+
## JSON Unmarshaling Safety
152+
153+
**Wrong - API might return number as ID:**
154+
```go
155+
type User struct {
156+
ID string `json:"id"` // Fails if API returns {"id": 12345}
157+
}
158+
```
159+
160+
**Correct - flexible type:**
161+
```go
162+
type User struct {
163+
ID json.Number `json:"id"` // Handles both "12345" and 12345
164+
}
165+
166+
// Usage
167+
userID := user.ID.String()
168+
```
169+
170+
---
171+
172+
## Detection in Code Review
173+
174+
**Red flags:**
175+
1. `resp.Body` or `resp.StatusCode` in error path without nil check
176+
2. `defer resp.Body.Close()` before error check
177+
3. Direct type assertions `x.(type)` without ok check
178+
4. Direct `.ParentResourceId.Resource` access
179+
5. `ID string` for fields that might be numbers
180+
181+
**Questions to ask:**
182+
- "What if resp is nil here?"
183+
- "What if this key is missing from the map?"
184+
- "What if ParentResourceId is nil?"

0 commit comments

Comments
 (0)