fix: close HTTP response bodies in linear client#52
Conversation
Each GraphQL helper that returned *http.Response forced its callers to close the body; none did. Drop the *http.Response return value and have each helper close its body internally via a shared closeResponse helper. Eliminates 14 bodyclose lint failures across the connector package without changing public connector behavior.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| func closeResponse(resp *http.Response) { | ||
| if resp != nil && resp.Body != nil { | ||
| _ = resp.Body.Close() | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: Several other methods in this file (AddMemberToTeam, CreateOrganizationInvite, SuspendUser, RemoveTeamMembership, CreateIssue, BulkCreateIssues, GetIssue) still use defer resp.Body.Close() after the error check. If doRequest ever returns both a non-nil response and an error, those bodies will leak. Consider applying the same closeResponse pattern here for consistency.
Connector PR Review: fix: close HTTP response bodies in linear clientBlocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0 Review SummaryThe new commit applies Security IssuesNone found. Correctness IssuesNone found. SuggestionsNone. |
The mutation helpers were deferring Body.Close after the err check, so when doRequest returned a non-nil response together with an err (notably the 400/429 rate-limit path), the body leaked. Move defer closeResponse above the err check across AddMemberToTeam, RemoveTeamMembership, CreateOrganizationInvite, SuspendUser, CreateIssue, BulkCreateIssues, and GetIssue for consistency with the read helpers.
madison-c-evans
left a comment
madison-c-evans
left a comment
There was a problem hiding this comment.
LGTM. Nice catch — this is actually a real bug fix on top of the lint cleanup: doRequest returns a non-nil resp along with a non-nil err on Linear's rate-limit path (the 400→429 rewrite at client.go:1075-1080), so under the old defer resp.Body.Close()-after-error-check pattern, bodies leaked on every 429. Moving the defer (via the nil-safe closeResponse helper) before the error check fixes that.
Verified locally: go build, go vet, go test ./... all clean. Every *http.Response removed from the public signatures was already being discarded with _ at every callsite, so the API simplification is risk-free.
2 participants
Summary
*http.Responsefrom the return signatures of the Linear GraphQL helpers (GetUsers,GetTeams,GetProjects,GetOrganization,GetTeam,GetProject,Authorize,GetTeamMemberships,ListTeamWorkflowStates,ListIssueFields,ListIssuesByIDs,GetIssueLabel,CreateIssueLabel). Each helper now deferscloseResponse(resp)internally — none of the connector callsites used the response object anyway.bodycloselint failures across the connector package.golangci-lint runis now 0 issues.Test plan
go test ./...passes.golangci-lint run --timeout=10m ./...reports 0 issues.