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

Request timeout option is broken #843

Closed
jerwen opened this issue Jul 26, 2023 · 1 comment · Fixed by #850
Closed

Request timeout option is broken #843

jerwen opened this issue Jul 26, 2023 · 1 comment · Fixed by #850
Assignees
Labels
Good First Issue Good First Issue

Comments

@jerwen
Copy link

jerwen commented Jul 26, 2023

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

  • SDK Version: 3.2.0
  • Node Version: 18.15.0
  • Operating System and version: alpine:3
@manchuck manchuck self-assigned this Jul 26, 2023
@manchuck manchuck added the Good First Issue Good First Issue label Jul 26, 2023
@jerwen
Copy link
Author

jerwen commented Aug 23, 2023

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 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue Good First Issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants