alpacahq / alpacahq/alpaca-trade-api-go
CancelOrder/CancelAllOrders never close the response body on success (missing defer closeResp)
- Lenguaje dominante
- Go
- Estrellas
- 429
- Forks
- 118
- Métricas de merge de PR
- Sin PR fusionados en 30 d
Descripción
v3.11.0. [`CancelOrder`](https://github.com/alpacahq/alpaca-trade-api-go/blob/5d728f80dc5ac823b76c7c881fa8475e268d6fe8/alpaca/rest.go#L667-L680) and [`CancelAllOrders`](https://github.com/alpacahq/alpaca-trade-api-go/blob/5d728f80dc5ac823b76c7c881fa8475e268d6fe8/alpaca/rest.go#L682-L694) are the only REST methods that don't close the response body on the success path. Every sibling does `defer closeResp(resp)`; these two `return verify(resp)` instead, and [`verify`](https://github.com/alpacahq/alpaca-trade-api-go/blob/5d728f80dc5ac823b76c7c881fa8475e268d6fe8/alpaca/rest.go#L1426-L1432) closes only when the status is >= 300.
```go
// CancelOrder; CancelAllOrders is the same shape
resp, err := c.delete(u)
if err != nil {
return err
}
return verify(resp) // no defer closeResp(resp)
func verify(resp *http.Response) error {
if resp.StatusCode >= http.StatusMultipleChoices {
defer resp.Body.Close()
return APIErrorFromResponse(resp)
}
return nil // success: body neither read nor closed
}
```
[`defaultDo` already calls `verify` before returning](https://github.com/alpacahq/alpaca-trade-api-go/blob/5d728f80dc5ac823b76c7c881fa8475e268d6fe8/alpaca/rest.go#L117-L119), so the status here is always < 300 and this `verify` can never fire. Looks like the `defer closeResp(resp)` was just left out rather than replaced by something.
Measured against a fixture returning 204 for `DELETE /v2/orders/{id}` and 207 + JSON for `DELETE /v2/orders`, with an `httptrace` `GotConn.Reused` probe on the following request: after `CancelAllOrders` the connection is not pooled and the next request pays a fresh TCP + TLS handshake. `CancelOrder` does get reuse, but only because `readLoop` pools a bodyless response's connection without waiting for a body read (`hasBody := ... && resp.ContentLength != 0` in `net/http/transport.go`), so it would break if that endpoint ever answered with a body.
Both also leak the `http.Client.Timeout` watchdog goroutine on any status, since `cancelTimerBody.stop` is only called from `Close`: every successful cancel strands a goroutine for the full timeout, 10s with the SDK's default client.
Happy to send a PR.
Guía de contribución
No hay ninguna guía de contribución indexada para este repositorio
Evaluación
Este issue todavía no se ha evaluado.