Skip to content

chore(lint): clear the lint findings from the client refresh change - #111

Merged
juicycleff merged 1 commit into
mainfrom
fix/client-refresh-lint
Sep 6, 2026
Merged

juicycleff merged 1 commit into
mainfrom
fix/client-refresh-lint

Conversation

@juicycleff

Copy link
Copy Markdown
Contributor

ci / Lint has been red on main since the client refresh work landed. This clears it. No behaviour changes, and one new test.

The bodyclose hits were not what they looked like

CI named three bodyclose findings in client_autorefresh_test.go. The helpers on those lines already close the body, from t.Cleanup. bodyclose cannot see through a function that returns *http.Response, so it blames whoever calls it.

Eight callers matched, not three. golangci-lint reports at most three of the same issue by default, so patching the named lines would have handed you the next three on the following run. response_delivery_test.go has the same helper shape and was sitting entirely behind that cap.

The helpers now return the response facts the tests assert on, and close the body where it is read. Nothing in any of these tests ever touched the body.

noctx

anonRequest went through http.Client.Get, which carries no context. It builds its request the way cookieRequest and bearerRequest already do.

unparam, and a decision you may want to reverse

Every call site handed refreshRouter a 5 minute threshold, so unparam called the parameter dead weight. Two ways to silence that: delete the parameter, or use it. This takes the second.

There is now a test where the same 50 minute session that SkipsWhenFarFromExpiry leaves alone gets rotated under a 60 minute threshold. That is worth slightly more than the lint fix, because nothing previously proved AutoRefreshConfig.Threshold was read at all. The test discriminates: hardcode the threshold inside refreshRouter and it fails. If you would rather just drop the parameter, say so and it can go.

errcheck

errcheck runs with check-blank, so a bare _ = json.Unmarshal(...) does not satisfy it. It carries its reason now, matching the io.ReadAll line directly above it.

Verified

  • golangci-lint run ./... clean, and clean again under --max-same-issues=0 --max-issues-per-linter=0, so nothing is hiding behind the cap
  • 98 packages pass, the same count main was passing
  • go test -race on the touched tests

The request helpers in these two test files hand back an *http.Response and
close its body from t.Cleanup. bodyclose cannot see through the function
boundary, so it blamed all eight callers instead. Only three were reported,
because max-same-issues stops at three, which is why fixing the named lines
would have surfaced the next batch on the following run.

The helpers now return the response facts the tests actually assert on, and
close the body where it is read. Nothing reads the body in any of these tests.

anonRequest went through http.Client.Get, which has no context, so it now
builds a request the same way its two siblings do.

refreshThreshold was passed 5 minutes at every call site. Rather than delete
the parameter, there is now a test that pins it: the same 50 minute session
that is left alone under a 5 minute threshold gets rotated under a 60 minute
one. Nothing proved the configured value was read before.

errcheck runs with check-blank, so the bare _ = on json.Unmarshal was not
enough on its own. It carries the reason now, matching the line above it.
@juicycleff
juicycleff merged commit 769c94b into main Sep 6, 2026
18 checks passed
@juicycleff
juicycleff deleted the fix/client-refresh-lint branch September 6, 2026 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant