alpacahq / alpacahq/alpaca-trade-api-go
CancelOrder/CancelAllOrders never close the response body on success (missing defer closeResp)
- Langage dominant
- Go
- Étoiles
- 429
- Forks
- 118
- Métriques de merge des PR
- Aucune PR mergée en 30 j
Description
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.
Guide de contribution
Aucun guide de contribution indexé pour ce dépôt
Piste de recherche
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.
Rédigé par le modèle d'indexation à partir du texte de l'issue.
Évaluation
- Stack technique
- go
- Domaine
- api
- Type d'issue
- Bug
- Difficulté
- 1/5
- Temps estimé
- Moins d'une heure
- Activité
- Calme
- Clarté
- Clairement spécifiée
- Accessibilité débutants
- 90/100