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.

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
Changes from 1 commit
Commits
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
23 changes: 21 additions & 2 deletions src/main-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -782,9 +782,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