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

A customer reported that it's not possible to set the HTTP Timeout at all now. #127

Closed
lukeocodes opened this issue Sep 5, 2023 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@lukeocodes
Copy link
Contributor

          A customer reported that it's not possible to set the HTTP Timeout at all now.

When they try to set it to 300 it times out at 100

The timeout should be set as 300 seconds as per the following code and I believe I can confirm the timeout by printing it out to the log:

deepgram.SetHttpClientTimeout(TimeSpan.FromSeconds(ApiRequestTimeoutInSeconds));

_logger.LogInformation($"ApiRequestTimeoutInSeconds: {ApiRequestTimeoutInSeconds.ToString()}");

And the output from the log file:

2023-08-30 07:32:18.818 +10:00 [INF] BinConsulting.TOPS.TOPService.AsrWorker: ApiRequestTimeoutInSeconds: 300

Error:

Error while transcribing audio files: The request was canceled due to the configured HttpClient.Timeout of 100 seconds elapsing.
DeepgramClient deepgram =
    new DeepgramClient(
        new Credentials(
            ApiKey,
            ApiUrl,
            ApiUrlRequiresSsl));
deepgram.SetHttpClientTimeout(TimeSpan.FromSeconds(300));
transcript = await deepgram.Transcription.Prerecorded.GetTranscriptionAsync(
            new StreamSource(stream, "audio/mpeg"), 
            new PrerecordedTranscriptionOptions() 
            {
                Tier = "enhanced",
                //Model = "meeting",
                Version = "latest",
                Language = "en",
                Diarize = options.EnableSpeakerSeparation,
                NamedEntityRecognition = true,
                Numerals = true,
                Punctuate = true,
                Utterances = true,
            });

Originally posted by @DamienDeepgram in #116 (comment)

@lukeocodes lukeocodes added the bug Something isn't working label Sep 5, 2023
@lukeocodes
Copy link
Contributor Author

@ThindalTV this appears to be related to your last change on timeouts. Any chance you could take a look at it?

@DamienDeepgram
Copy link
Contributor

I tested by setting a 1 second timeout and a large 180mb MP3 file and the request did not timeout so it seems that something is not quite right

@ThindalTV
Copy link
Collaborator

It is most probably a bug as the 100s timeout is the default value for the timeout. I will take a look.

@andyr700
Copy link

andyr700 commented Oct 25, 2023

I am having this same issue, SetHttpClientTimeout just doesn't take effect.

Update:
I had to hotfix the Deepgram SDK so we can continue progressing internally. The problem is that the ApiRequest object took the HttpClient in its constructor which creates a pointer to the original HttpClient (see BaseClient constructor). When the SetTimeOut method in HttpClientUtil calls Create() again it creates a new HttpClient in a new location in memory. But the ApiRequest methods will all continue using the pointer to the original HttpClient it received in its ctor. I've no history on the other ticket but for the moment deleting the call to Create() in SetTimeOut would mean it updates the HttClient used by the ApiRequest object.

@jpvajda jpvajda added this to the .Net SDK v4 Release milestone Nov 1, 2023
@jpvajda
Copy link
Contributor

jpvajda commented Nov 1, 2023

The team discussed this and in our next major release of the .NET SDK this issue will be addressed.

CopperBeardy added a commit that referenced this issue Nov 13, 2023
Refactored the AbstractRestClient so that it can either accept a HttpClient or a IHttpClientFactory. if  a HttpClient is passed in it is stored in the parameter ExternalHttpClient. During a Rest call the methods will checker wheter a HttpClient or a IHttpClientFactory is present then assign or create HttpClient that will make the rest call, this allows for a timeout to be set at anytime. this will address both issues -
Timeout -#127
Socket Exhaustion - #148
@dhymik
Copy link
Contributor

dhymik commented Dec 30, 2023

@andyr700

...for the moment deleting the call to Create() in SetTimeOut would mean it updates the HttClient used by the ApiRequest object.

Thank you! This helped me fix this bug for now, see this pull request #179 and this branch in my fork.

@davidvonthenen
Copy link
Contributor

This PR #207 has been merged and released 3.4.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants