Hacktoberfest 2026: as issues que os mantenedores marcaram para outubro, abertas e boas para iniciantes. Ver issues do Hacktoberfest

Rate-limit retry replays a consumed request body for POST/PUT/PATCH

Aberta
  • Algo errado?
#4,656 1 comentário 0 reações 0 responsáveis Ver no GitHub

Mantenedores costumam responder em até 1 dia

@AdamMagued já está trabalhando nisso.

Desde 21/10/2026.

  • #4657 de @AdamMagued — aberto
  • #4658 de @sai5880 — fechado sem integrar

Avaliação

Dificuldade
3/5
Tempo estimado
1-2 dias
Facilidade para iniciantes
70/100
Tipo de issue
Bug
Clareza
Claramente especificada
Status de atividade
Ativa
Stack de tecnologia
go
Domínio
backend-api-design

Direção de pesquisa

Start with the retry branch in github/github.go at lines 1521–1529, then read NewRequest and the existing retry tests in github/github_test.go named in the issue. Add regression coverage for POST, PUT, and PATCH bodies and for a non-replayable body. Done means retry requests preserve replayable bodies and requests without a replayable body are not retried.

Escrita pelo modelo de indexação a partir do texto da issue.

Descrição

Summary

The automatic primary rate-limit retry path in Client.bareDo re-sends a cloned *http.Request without restoring its body. For any request that carries a body (POST, PUT, PATCH), the original req.Body has already been consumed by the first caller.Do(req) call, so the retry sends an empty body or fails on ContentLength.

Location

github/github.go:1521-1529 (retry branch in bareDo), with request construction at github/github.go:993-1037 (NewRequest) and the send at github/github.go:1464 (caller.Do(req)).

var rateLimitError *RateLimitError
if errors.As(err, &rateLimitError) &&
    ctx.Value(SleepUntilPrimaryRateLimitResetWhenRateLimited) != nil {
    if err := sleepUntilResetWithBuffer(ctx, rateLimitError.Rate.Reset.Time); err != nil {
        return response, err
    }
    // retry the request now the rate limit should have been reset
    newReq := req.Clone(context.WithValue(ctx, SleepUntilPrimaryRateLimitResetWhenRateLimited, nil))
    return c.bareDo(caller, newReq)
}

Details

NewRequest encodes the body into a *bytes.Buffer:

var buf io.ReadWriter
if body != nil {
    buf = &bytes.Buffer{}
    enc := json.NewEncoder(buf)
    enc.SetEscapeHTML(false)
    err := enc.Encode(body)
    if err != nil {
        return nil, err
    }
}

req, err := http.NewRequestWithContext(ctx, method, u.String(), buf)

For the *bytes.Buffer case, net/http.NewRequestWithContext populates req.GetBody and sets req.ContentLength. However, bareDo never uses GetBody. It calls caller.Do(req) at line 1464, which reads req.Body to EOF (and closes it). When a *RateLimitError triggers the retry branch, req.Clone(...) performs a shallow copy: the cloned request keeps the same consumed *http.Request.Body reader and the original ContentLength. The retry therefore either sends a zero-length body while still advertising the original ContentLength, or returns an error such as http: ContentLength=N with Body length 0.

The fix belongs in this retry path (or in bareDo before every send): call req.GetBody() and assign the resulting reader to req.Body before re-sending. If GetBody is nil (a body that cannot be replayed, for example a stream passed to NewFormRequest), the retry must not be attempted, because there is no way to reconstruct the payload.

Impact

  • Users who opt into SleepUntilPrimaryRateLimitResetWhenRateLimited and then perform a write operation (POST/PUT/PATCH) can have that write silently truncated to an empty body after a primary rate-limit retry. This is a correctness issue and can cause confusing API errors or, depending on the endpoint, unintended empty updates.
  • The retry can also fail outright with a ContentLength mismatch error instead of succeeding on the second attempt.
  • GET requests are unaffected, which is why existing tests do not detect this.

Suggested fix

In the retry branch, restore the body from GetBody before cloning or re-sending:

newReq := req.Clone(context.WithValue(ctx, SleepUntilPrimaryRateLimitResetWhenRateLimited, nil))
if req.GetBody != nil {
    body, err := req.GetBody()
    if err != nil {
        return response, err
    }
    newReq.Body = body
} else if req.Body != nil && req.Body != http.NoBody {
    // Body cannot be replayed; do not retry.
    return response, err
}
return c.bareDo(caller, newReq)

Applying the restore inside bareDo before each caller.Do would also protect other re-send paths, but the minimal change is the rate-limit retry branch.

Verification

  • Static review of github/github.go at HEAD 2190e6a (2026-10-22): NewRequest encodes into bytes.Buffer, bareDo calls caller.Do once and then req.Clone on retry, and no GetBody restoration exists in github/github.go.
  • Existing retry coverage is GET-only. The SleepUntilPrimaryRateLimitResetWhenRateLimited tests in github/github_test.go (lines 3033, 3066, 3095, 3150, 3201) all construct requests with "GET" and nil body, so they never exercise body replay.
  • Suggested regression tests: a table test using POST, PUT, and PATCH where the handler returns a *RateLimitError (403 with rate-limit headers) on the first call and 200 on the second, asserting the second request receives the original JSON body. Add a case for a non-replayable body to assert the retry is skipped.
  • This report is based on static source review. No runtime proof of concept was executed.
Linguagem predominante
Go
Estrelas
11.3k
Forks
2.6k
Merge médio
2d 8h
PRs com merge (30d)
38

Preparar o ambiente

Primeiros passos

  1. Leia a issue inteira e depois o guia de contribuição do projeto.
  2. Comente na issue dizendo que vai assumir — evita que duas pessoas façam o mesmo trabalho.
  3. Faça um fork do repositório e trabalhe em uma branch.
  4. Abra um pull request que referencie o número da issue.

Mais de google/go-github

Todas as issues de google/go-github

Issues semelhantes

Mais issues de Go

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.