Vonage / vonage-node-sdk

Vonage API client for Node.js. API support for SMS, Voice, Text-to-Speech, Numbers, Verify (2FA) and more.
Apache License 2.0
375 stars 178 forks source link

fix(subaccounts): correct incorrect type on `BalanceTransferParameters` #937

Closed froggy1014 closed 2 weeks ago

froggy1014 commented 2 weeks ago

Description

The transferBalance method in the SubAccounts class expects parameters defined by the BalanceTransferParameters type:

export type BalanceTransferParameters = {
    /**
     * The account ID from which the balance is transferred.
     */
    from: string;
    /**
     * The account ID to which the balance is transferred.
     */
    to: string;
    /**
     * The amount of balance to transfer.
     */
    amount: string;
    /**
     * (Optional) A reference for the balance transfer.
     */
    reference?: string;
};

transferBalance(params: BalanceTransferParameters): Promise<BalanceTransfer>;

Notably, the amount field is defined as a string type.

Problem

When invoking transferBalance with the amount as a string, a 422 Unprocessable Entity error is thrown :

await this.subAccounts
      .transferBalance({
        from: primaryAccountId,
        to: subAccountId,
        amount: '0.01'
      })
      .then((res) => console.log(res))
      .catch(this.handleError)

Error :

VetchError: Request failed with status code 422
    at SubAccounts.parseResponse (/node_modules/@vonage/server-client/dist/client.js:304:19)
    at SubAccounts.sendRequest (/node_modules/@vonage/server-client/dist/client.js:222:31)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async SubAccounts.sendRequestWithData (/node_modules/@vonage/server-client/dist/client.js:197:16)
    at async SubAccounts.transferBalance (/node_modules/@vonage/subaccounts/dist/subaccount.js:110:22)
{
  config: {
    url: 'https://api.nexmo.com/accounts/e947ef5c/balance-transfers',
    method: 'POST',
    type: 'application/json',
    data: {
      from: 'PRIMARY_ACCOUNT_ID',
      to: 'SUB_ACCOUNT_ID',
      amount: '0.01',
    },
    headers: {
      'user-agent': '@vonage/server-sdk/3.0.0, node/20.12.2,',
      'content-type': 'application/json',
      Authorization: 'Basic ~'
    }
  },
  response: Response {
    size: 0,
    timeout: 0,
    [Symbol(Body internals)]: { body: [PassThrough], disturbed: false, error: null },
    [Symbol(Response internals)]: {
      url: 'https://api.nexmo.com/accounts/PRIMARY_ACCOUNT_ID/balance-transfers',
      status: 422,
      statusText: 'Unprocessable Entity',
      headers: [Headers],
      counter: 0
    }
  }
}

Solution

When the amount is provided as a number, the request works correctly :

await this.subAccounts
      .transferBalance({
        from: primaryAccountId,
        to: subAccountId,
        amount: 0.01
      } as any )
      .then((res) => console.log(res))
      .catch(this.handleError)

Successful Response:

{
  masterAccountId: 'PRIMARY_ACCOUNT_ID',
  links: {
    self: {
      href: '/accounts/PRIMARY_ACCOUNT_ID/balance-transfers/TRANSFER_ID'
    }
  },
  from: 'PRIMARY_ACCOUNT_ID',
  to: 'SUB_ACCOUNT_ID',
  amount: '0.01',
  reference: null,
  id: 'TRANSFER_ID',
  createdAt: '2024-06-11T05:24:41.000Z'
}

Conclusion

The BalanceTransferParameters type definition for the amount field appears to be incorrect. Despite being defined as a string, it needs to be passed as a number for the request to succeed. This discrepancy should be addressed to avoid confusion and potential errors.

[AS-IS]

export type BalanceTransferParameters = {
    from: string;
    to: string;
    amount: string;
    reference?: string;
};

[TO-BE]

export type BalanceTransferParameters = {
    from: string;
    to: string;
    amount: number;
    reference?: string;
};

Motivation and Context

Testing Details

Example Output or Screenshots (if appropriate)

Types of changes

Checklist

manchuck commented 2 weeks ago

Another good catch. I'm not sure how that came about