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

Request timeout option is broken #843

Closed jerwen closed 11 months ago

jerwen commented 11 months ago

I'm trying to interrupt long request by leveraging the timeout option available on the Vonage client object but it does not behave as expected.

Expected Behavior

The request should throw a timeout exception when said request is taking too much time.

Current Behavior

No exception is raised and the request keeps running beyond the timeout limitation.

Possible Solution

The timeout option is explicitly ignored in the code as you can see here

Steps to Reproduce (for bugs)

Here is a code snippet describing the usage and the expected behavior

import { Auth } from '@vonage/auth';
import { Vonage } from '@vonage/server-sdk';
import type { SMSMessages } from '@vonage/sms/dist/types';

const requestTimeout = 1; // in milliseconds I supposed. It is nowhere to be found in documentation
const client: Vonage = new Vonage(
  new Auth({
    apiKey: <my-api-key>,
    apiSecret: <my-api-secret>,
  }),
  {
    responseType: undefined,
    restHost: <vonage-api-host>,
    timeout: requestTimeout, // This timeout option is being completely ignored
    videoHost: undefined,
    apiHost: undefined,
  },
);

const sendSMS = (phone: string, message: string, from: string): Promise<SMSMessages> =>
{
  return client.sms.send({
    to: phone,
    from,
    text: message,
    clientRef: config.clientName,
  });
}

// This following call should fail due to timeout parameter being provided with a small value
// But it always succeeds, completely ignoring the timeout option provided to the client
sendSMS('1233456987', 'Message that shouldn't get sent because of timeout', 'Timeout')
  .then((res) => console.log(res))
  .catch((error) => console.log(error));

Context

We are trying to avoid SMS sending jobs locking our dedicated fast running worker due to requests taking too much time to be executed.

Your Environment

jerwen commented 10 months ago

Thank you @manchuck for the fix at the library level. Unfortunately the fix seems incomplete as from the client side, the timeout configuration option is explicitly set to null as you can see here. Is it possible for you to set that configuration ?