Skip to content

fix: send no request body when body is nil#2

Merged
AcidSailor merged 1 commit into
mainfrom
fix/omit-nil-request-body
Jul 5, 2026
Merged

fix: send no request body when body is nil#2
AcidSailor merged 1 commit into
mainfrom
fix/omit-nil-request-body

Conversation

@AcidSailor

Copy link
Copy Markdown
Owner

What

do() marshalled a nil body to the JSON literal null and always attached it via bytes.NewReader, so every call — including body-less GETs — shipped a 4-byte null request body. Some servers/CDNs reject a GET that carries a body (this surfaced while migrating sponsrdownloader to restkit, where every paginated GET began sending null).

How

Marshal the body (and set Content-Type: application/json) only when body != nil. A nil body now maps to a nil io.Reader, which http.NewRequestWithContext treats as no body (Body == nil, ContentLength == 0, GetBody == nil).

Behaviour is unchanged for non-nil bodies. Note: a typed nil (e.g. (*T)(nil)) is still a non-nil any and marshals to null as before — only the untyped nil literal, the common case, is affected.

Testing

  • task ci → 0 issues; go test ./... → 18 passed.
  • New: TestDo_NilBodySendsNoRequestBody (nil → empty body, no Content-Type, zero Content-Length) and TestDo_NonNilBodyIsSent (non-nil body still sent with Content-Type).

🤖 Generated with Claude Code

do() marshalled a nil body to the JSON literal `null` and always attached
it via bytes.NewReader, so every call — including body-less GETs — shipped
a 4-byte `null` request body. Some servers/CDNs reject a GET that carries a
body.

Now marshal (and set Content-Type) only when body != nil; a nil body maps
to a nil io.Reader, which http.NewRequestWithContext treats as no body.

Adds TestDo_NilBodySendsNoRequestBody and TestDo_NonNilBodyIsSent.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@AcidSailor AcidSailor marked this pull request as ready for review July 5, 2026 08:31
@AcidSailor AcidSailor merged commit 20c03f3 into main Jul 5, 2026
2 checks passed
@AcidSailor AcidSailor deleted the fix/omit-nil-request-body branch July 5, 2026 08:31
AcidSailor added a commit to AcidSailor/sponsrdownloader that referenced this pull request Jul 5, 2026
restkit v0.1.3 no longer attaches a `null` body to body-less GETs
(AcidSailor/restkit#2), so the local stripBodyHook workaround is
redundant. Remove the hook and its registrations; keep
TestGetObjects_OmitsRequestBody as a regression guard that our GETs stay
body-less, now backed by restkit rather than the hook.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
AcidSailor added a commit to AcidSailor/sponsrdownloader that referenced this pull request Jul 5, 2026
* docs: design for migrating sponsr HTTP transport to restkit

* docs: implementation plan for restkit migration

* refactor(sponsr): extract rate-limit + 429 retry into a RoundTripper

* feat(sponsr): migrate JSON transport to restkit

* docs: remove restkit migration spec and plan

* fix(sponsr): address PR review feedback

- Strip restkit's default `null` GET body via a stripBodyHook so requests
  reach sponsr.ru body-less, as before the migration (Sponsr's API/CDN may
  reject a GET carrying a body). Covered by TestGetObjects_OmitsRequestBody.
- Make rateLimitRetryTransport own its numeric invariants: the constructor
  now clamps retryBaseDelay <= 0 to the default (avoids a zero-backoff
  busy-spin) and floors maxRetries at 0. Rename the base-delay const to
  defaultRetryBaseDelay. Covered by TestNewTransport_ClampsInvalidParams.
- Stop RoundTrip mutating the caller's request: a replay now runs on a
  req.Clone with its body rewound, per the http.RoundTripper contract.
- Restore lost/absent coverage: 429->200->decode recovery
  (TestGetObjects_RecoversAfter429), non-429 not retried
  (TestTransport_DoesNotRetryNon429), and the HTML-scrape non-200 path
  (TestProjectIDBySlug_HTTPError).
- Tighten comments (bearerAuthHook/httpClient scope, drain-error discard)
  and refresh CLAUDE.md's stale doRequest description to point at
  transport.go + restkit.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

* chore(deps): bump restkit to v0.1.3, drop stripBodyHook

restkit v0.1.3 no longer attaches a `null` body to body-less GETs
(AcidSailor/restkit#2), so the local stripBodyHook workaround is
redundant. Remove the hook and its registrations; keep
TestGetObjects_OmitsRequestBody as a regression guard that our GETs stay
body-less, now backed by restkit rather than the hook.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
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