alpacahq / alpacahq/alpaca-trade-api-go

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

Abierto Apto para principiantes
#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

Línea de trabajo

Start in alpaca/rest.go at CancelOrder and CancelAllOrders, then compare their response handling with sibling REST methods and inspect verify. Confirm that successful cancel responses release the response body while error handling remains unchanged; the issue’s 204 and 207 fixtures and connection-reuse probe describe the expected behavior.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
go
Área
api
Tipo de issue
Error
Dificultad
1/5
Tiempo estimado
Menos de una hora
Estado de actividad
Tranquilo
Claridad
Bien especificado
Aptitud para principiantes
90/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.