Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "array of symbols" signature to getSymbolPriceTicker #393

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Add "array of symbols" signature to getSymbolPriceTicker #393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
de67e64
Add "array of symbols" signature to `getSymbolPriceTicker`
futpib Feb 3, 2024
4652f44
2.9.5
futpib Feb 4, 2024
e375be8
Merge branch 'master' into getSymbolPriceTicker-symbols
tiagosiebler Mar 4, 2024
5342cb7
Update package.json
tiagosiebler Mar 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "binance",
"version": "2.10.2",
"version": "2.10.3",
"description": "Node.js & JavaScript SDK for Binance REST APIs & WebSockets, with TypeScript & end-to-end tests.",
"main": "lib/index.js",
"types": "lib/index.d.ts",
Expand Down
23 changes: 21 additions & 2 deletions src/main-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -785,9 +785,28 @@ export class MainClient extends BaseRestClient {
}

getSymbolPriceTicker(
params?: Partial<BasicSymbolParam>,
params: BasicSymbolParam,
): Promise<SymbolPrice>;

getSymbolPriceTicker(
params?: SymbolArrayParam,
): Promise<SymbolPrice[]>;

getSymbolPriceTicker(
params?: Partial<BasicSymbolParam> | Partial<SymbolArrayParam>,
): Promise<SymbolPrice | SymbolPrice[]> {
return this.get('api/v3/ticker/price', params);
if (params && typeof params['symbol'] === 'string') {
return this.get('api/v3/ticker/price', params);
}

if (params && params['symbols'] && Array.isArray(params['symbols'])) {
const symbols = (params as SymbolArrayParam).symbols;
const symbolsQueryParam = JSON.stringify(symbols);

return this.get('api/v3/ticker/price?symbols=' + symbolsQueryParam);
}
Comment on lines +802 to +807
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GET requests are automatically mapped to a query string, does this not work if you simply pass the object too - similar to your change at line 795?

      return this.get('api/v3/ticker/price', params);

Nothing wrong with what you did, just checking in case you haven't tried it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it, you really have to put json array string inside the query string here just like in api/v3/ticker/24hr.

You can check this on the master branch:

$ node -e "Object.assign(global, require('.')); mc = new MainClient(); [x => x, JSON.stringify].map(f => mc.getSymbolPriceTicker({ symbols: f(['BTCUSDT', 'ETHUSDT']) }).then(console.log, console.error));"
 
[
  { symbol: 'BTCUSDT', price: '42954.95000000' },
  { symbol: 'ETHUSDT', price: '2305.66000000' }
]
{
  code: -1101,
  message: "Duplicate values for parameter 'symbols[]'.",
  body: { code: -1101, msg: "Duplicate values for parameter 'symbols[]'." },
  headers: Object [AxiosHeaders] {
    'content-type': 'application/json;charset=UTF-8',
    'content-length': '66',
    connection: 'keep-alive',
    date: 'Sun, 04 Feb 2024 17:13:47 GMT',
    server: 'nginx',
    'x-mbx-uuid': 'a75326aa-3fcb-4788-bc8f-d097b07719a0',
    'x-mbx-used-weight': '10',
    'x-mbx-used-weight-1m': '10',
    'strict-transport-security': 'max-age=31536000; includeSubdomains',
    'x-frame-options': 'SAMEORIGIN',
    'x-xss-protection': '1; mode=block',
    'x-content-type-options': 'nosniff',
    'content-security-policy': "default-src 'self'",
    'x-content-security-policy': "default-src 'self'",
    'x-webkit-csp': "default-src 'self'",
    'cache-control': 'no-cache, no-store, must-revalidate',
    pragma: 'no-cache',
    expires: '0',
    'x-cache': 'Error from cloudfront',
    via: '1.1 4fa064f65088b74bd9abffd69e1e9de4.cloudfront.net (CloudFront)',
    'x-amz-cf-pop': 'SOF50-P2',
    'x-amz-cf-id': 'T4Lbxi-PETLyhXlrsf1xUYwS30xrWsVLxFsoWHmfl7saV8Rt_9Ok5g=='
  },
  requestUrl: 'https://api.binance.com/api/v3/ticker/price',
  requestBody: undefined,
  requestOptions: {
    recvWindow: 5000,
    syncIntervalMs: 3600000,
    strictParamValidation: false,
    disableTimeSync: true,
    api_key: undefined,
    api_secret: undefined
  }
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, I guess it does need to be stringified. One last request, can you see if this works (instead of building the query string in the URL)?

    if (params && params['symbols'] && Array.isArray(params['symbols'])) {
      return this.get('api/v3/ticker/price', {
        symbols: JSON.stringify(params['symbols']),
      });
    }

This should automatically handle building the request properly on the networking layer of the SDK, instead of building a query string here. Slight nitpick but it's consistent with the rest of the endpoints. Once that's confirmed working and added to the PR, I can merge this. Thanks!

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@futpib in case you missed it ^


return this.get('api/v3/ticker/price');
}

getSymbolOrderBookTicker(
Expand Down