FIX: origClientOrderIdList format in BatchCancelOrders API#794
Conversation
|
@boyi thanks! |
There was a problem hiding this comment.
Pull request overview
This pull request fixes the format of the origClientOrderIdList parameter in the BatchCancelOrders API for futures trading. The previous implementation was incorrectly passing the slice directly to setFormParam, while the API expects a JSON-array-like string format with quoted string elements.
Key Changes:
- Converts the
origClientOrderIDListslice to a properly formatted string (e.g.,["my_id_1","my_id_2"]) - Adds string concatenation logic to wrap each element in double quotes and join them with commas
- Includes a descriptive comment explaining the format transformation
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| origClientOrderIDListString := "[\"" + strings.Join(s.origClientOrderIDList, "\",\"") + "\"]" | ||
| r.setFormParam("origClientOrderIdList", origClientOrderIDListString) |
There was a problem hiding this comment.
The string formatting logic doesn't handle empty slices correctly. When origClientOrderIDList is an empty slice (not nil, but length 0), this code will produce the string [""] instead of []. This happens because strings.Join on an empty slice returns an empty string, and the code then wraps it with [" + "" + "].
Consider checking the length of the slice before formatting, similar to how the nil check is done.
| origClientOrderIDListString := "[\"" + strings.Join(s.origClientOrderIDList, "\",\"") + "\"]" | |
| r.setFormParam("origClientOrderIdList", origClientOrderIDListString) | |
| if len(s.origClientOrderIDList) == 0 { | |
| r.setFormParam("origClientOrderIdList", "[]") | |
| } else { | |
| origClientOrderIDListString := "[\"" + strings.Join(s.origClientOrderIDList, "\",\"") + "\"]" | |
| r.setFormParam("origClientOrderIdList", origClientOrderIDListString) | |
| } |
| if s.origClientOrderIDList != nil { | ||
| r.setFormParam("origClientOrderIdList", s.origClientOrderIDList) | ||
| // convert a slice of strings to a string e.g. ["my_id_1","my_id_2"], encode the double quotes. No space after comma. | ||
| origClientOrderIDListString := "[\"" + strings.Join(s.origClientOrderIDList, "\",\"") + "\"]" |
There was a problem hiding this comment.
The formatting approach is inconsistent with the options API implementation in v2/options/order_service.go (lines 453-459), which uses a different pattern involving fmt.Sprintf to quote individual elements. While both approaches could work, consider aligning the implementation for consistency across the codebase. The options implementation uses fmt.Sprint on the slice which handles edge cases differently.
| origClientOrderIDListString := "[\"" + strings.Join(s.origClientOrderIDList, "\",\"") + "\"]" | |
| list := fmt.Sprint(s.origClientOrderIDList) // e.g. "[my_id_1 my_id_2]" | |
| list = strings.Trim(list, "[]") | |
| origClientOrderIDListString := "[]" | |
| if list != "" { | |
| items := strings.Split(list, " ") | |
| for i, v := range items { | |
| items[i] = fmt.Sprintf("%q", v) | |
| } | |
| origClientOrderIDListString = "[" + strings.Join(items, ",") + "]" | |
| } |
| } | ||
| if s.origClientOrderIDList != nil { | ||
| r.setFormParam("origClientOrderIdList", s.origClientOrderIDList) | ||
| // convert a slice of strings to a string e.g. ["my_id_1","my_id_2"], encode the double quotes. No space after comma. |
There was a problem hiding this comment.
The comment states "encode the double quotes," but the implementation doesn't actually encode them - it embeds literal escaped quotes in the string using backslash notation. If the API requires URL-encoded double quotes, consider using url.QueryEscape or clarifying the comment to accurately describe what the code does (e.g., "escape the double quotes with backslashes").
| // convert a slice of strings to a string e.g. ["my_id_1","my_id_2"], encode the double quotes. No space after comma. | |
| // convert a slice of strings to a JSON-style array string e.g. ["my_id_1","my_id_2"], escaping the double quotes with backslashes. No space after comma. |
| // convert a slice of strings to a string e.g. ["my_id_1","my_id_2"], encode the double quotes. No space after comma. | ||
| origClientOrderIDListString := "[\"" + strings.Join(s.origClientOrderIDList, "\",\"") + "\"]" | ||
| r.setFormParam("origClientOrderIdList", origClientOrderIDListString) |
There was a problem hiding this comment.
The string concatenation doesn't handle special characters that could appear in client order IDs. If an order ID contains a double quote character, the resulting string will be malformed. Consider using json.Marshal for proper escaping of special characters in the string values, or add validation/escaping for characters like quotes and backslashes.
| // convert a slice of strings to a string e.g. ["my_id_1","my_id_2"], encode the double quotes. No space after comma. | |
| origClientOrderIDListString := "[\"" + strings.Join(s.origClientOrderIDList, "\",\"") + "\"]" | |
| r.setFormParam("origClientOrderIdList", origClientOrderIDListString) | |
| // convert a slice of strings to a JSON array string, ensuring proper escaping of special characters | |
| origClientOrderIDListJSON, err := json.Marshal(s.origClientOrderIDList) | |
| if err != nil { | |
| return nil, err | |
| } | |
| r.setFormParam("origClientOrderIdList", string(origClientOrderIDListJSON)) |
No description provided.