From 3b1fe81d8bb2ab18d848fd97d455d7bf289a56b9 Mon Sep 17 00:00:00 2001
From: Yordan Miladinov <ydm@users.noreply.github.com>
Date: Sat, 31 Jul 2021 08:13:52 +0300
Subject: [PATCH] FTX: order cancellation improvement (#727)

* exchanges/request: Requester.doRequest() now always parses returned response body into JSON even in the case of an artificial error after the request itself

* exchanges/ftx: consider order cancellation successful under two new conditions, reported by the exchange: (1) order is already closed or (2) order is already queued for cancellation

* exchanges/ftx: fix a typo in a comment

* exchanges/request: keep the same behavior of doRequest() when there is an unmarshaling error

* exchanges/ftx: FTX.DeleteOrderByClientID now also reports no errors when requesting the cancellation of orders that are already canceled on the exchange

* exchanges/ftx: order deletion methods are now unified

* exchanges/ftx: DeleteOrder* methods now check if the given ID is not empty
---
 exchanges/ftx/ftx.go         | 51 ++++++++++++++++--------------------
 exchanges/request/request.go | 11 +++++---
 2 files changed, 29 insertions(+), 33 deletions(-)

diff --git a/exchanges/ftx/ftx.go b/exchanges/ftx/ftx.go
index 4b36972f..c9b061dd 100644
--- a/exchanges/ftx/ftx.go
+++ b/exchanges/ftx/ftx.go
@@ -129,6 +129,7 @@ const (
 )
 
 var (
+	errInvalidOrderID                                    = errors.New("invalid order ID")
 	errStartTimeCannotBeAfterEndTime                     = errors.New("start timestamp cannot be after end timestamp")
 	errSubaccountNameMustBeSpecified                     = errors.New("a subaccount name must be specified")
 	errSubaccountUpdateNameInvalid                       = errors.New("invalid subaccount old/new name")
@@ -776,51 +777,43 @@ func (f *FTX) GetOrderStatusByClientID(clientOrderID string) (OrderData, error)
 	return resp.Data, f.SendAuthHTTPRequest(exchange.RestSpot, http.MethodGet, getOrderStatusByClientID+clientOrderID, nil, &resp)
 }
 
-// DeleteOrder deletes an order
-func (f *FTX) DeleteOrder(orderID string) (string, error) {
+func (f *FTX) deleteOrderByPath(path string) (string, error) {
 	resp := struct {
 		Result  string `json:"result"`
 		Success bool   `json:"success"`
+		Error   string `json:"error"`
 	}{}
-	if err := f.SendAuthHTTPRequest(exchange.RestSpot, http.MethodDelete, deleteOrder+orderID, nil, &resp); err != nil {
-		return "", err
+	err := f.SendAuthHTTPRequest(exchange.RestSpot, http.MethodDelete, path, nil, &resp)
+	// If there is an error reported, but the resp struct reports one of a very few
+	// specific error causes, we still consider this a successful cancellation.
+	if err != nil && !resp.Success && (resp.Error == "Order already closed" || resp.Error == "Order already queued for cancellation") {
+		return resp.Error, nil
 	}
-	if !resp.Success {
-		return resp.Result, errors.New("delete order request by ID unsuccessful")
+	return resp.Result, err
+}
+
+// DeleteOrder deletes an order
+func (f *FTX) DeleteOrder(orderID string) (string, error) {
+	if orderID == "" {
+		return "", errInvalidOrderID
 	}
-	return resp.Result, nil
+	return f.deleteOrderByPath(deleteOrder + orderID)
 }
 
 // DeleteOrderByClientID deletes an order
 func (f *FTX) DeleteOrderByClientID(clientID string) (string, error) {
-	resp := struct {
-		Result  string `json:"result"`
-		Success bool   `json:"success"`
-	}{}
-
-	if err := f.SendAuthHTTPRequest(exchange.RestSpot, http.MethodDelete, deleteOrderByClientID+clientID, nil, &resp); err != nil {
-		return "", err
+	if clientID == "" {
+		return "", errInvalidOrderID
 	}
-	if !resp.Success {
-		return resp.Result, errors.New("delete order request by client ID unsuccessful")
-	}
-	return resp.Result, nil
+	return f.deleteOrderByPath(deleteOrderByClientID + clientID)
 }
 
 // DeleteTriggerOrder deletes an order
 func (f *FTX) DeleteTriggerOrder(orderID string) (string, error) {
-	resp := struct {
-		Result  string `json:"result"`
-		Success bool   `json:"success"`
-	}{}
-
-	if err := f.SendAuthHTTPRequest(exchange.RestSpot, http.MethodDelete, cancelTriggerOrder+orderID, nil, &resp); err != nil {
-		return "", err
+	if orderID == "" {
+		return "", errInvalidOrderID
 	}
-	if !resp.Success {
-		return resp.Result, errors.New("delete trigger order request unsuccessful")
-	}
-	return resp.Result, nil
+	return f.deleteOrderByPath(cancelTriggerOrder + orderID)
 }
 
 // GetFills gets fills' data
diff --git a/exchanges/request/request.go b/exchanges/request/request.go
index 5d707f66..da07ca53 100644
--- a/exchanges/request/request.go
+++ b/exchanges/request/request.go
@@ -197,6 +197,12 @@ func (r *Requester) doRequest(req *http.Request, p *Item) error {
 		if err != nil {
 			return err
 		}
+		// Even in the case of an erroneous condition below, yield the parsed
+		// response to caller.
+		var unmarshallError error
+		if p.Result != nil {
+			unmarshallError = json.Unmarshal(contents, p.Result)
+		}
 
 		if p.HTTPRecording {
 			// This dumps http responses for future mocking implementations
@@ -242,10 +248,7 @@ func (r *Requester) doRequest(req *http.Request, p *Item) error {
 					string(contents))
 			}
 		}
-		if p.Result != nil {
-			return json.Unmarshal(contents, p.Result)
-		}
-		return nil
+		return unmarshallError
 	}
 }