alpacahq / alpacahq/alpaca-trade-api-go

CancelOrder/CancelAllOrders never close the response body on success (missing defer closeResp)

Abierto
#374 0 comentarios 0 reacciones 0 asignados Ver en GitHub
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.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.