alpacahq / alpacahq/alpaca-trade-api-go

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

Ouverte Adaptée aux débutants
#374 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
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

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.