Missing functionality in CloudRecordings.DownloadFileAsync after PR 342 is merged

[Jericho]

PR #342 submitted by @pvgritsenko-ansible solves the "out of memory" problem when downloading large files, but the downside is that we lose some of the functionality built-in our library whenever we make API calls to the Zoom API.

Specifically:

• automatic retries when we get a HTTP 429 response
• error handling
• logging
• automatic token refresh when it expires
• fix for token being stripped when we get a HTTP 307 (as described in #257)

I want to think of a way to provide this functionality despite the fact that we invoke the Zoom endpoint in a unconventional way.

[Jericho]

Rather than invest time and effort in implementing this functionality, I decided to submit a PR to the FluentHttpClient project to add streaming ability to the http client. My PR was accepted and merged, I am now simply waiting for version 4.4 of that project to be released.

[Jericho]

@pvgritsenko-ansible My PR in the FluentHttpClient has been accepted and merged and a new version of the package was published. This means that we can take advantage of it to stream large responses when downloading a recording from Zoom while also benefiting from the functionality that is built-in this library such as automatic retries, error handling, logging, token refresh, etc.

I published a beta package to my personal NuGet feed called ZoomNet.0.77.0-pipeline-when-do0013.nupkg. Would you be able to test it before I publish a new release of ZoomNet?

[pvgritsenko-ansible]

@Jericho Great news! Yes I will test it within 3 days. I'll let you know about results.

[pvgritsenko-ansible]

@Jericho The first test showed that the DownloadFileAsync method returns System.NotSupportedException I'll test it deeper again tomorrow and notify you about results.

[Jericho]

The StackTrace in the screenshot you posted indicates that this exception occurred at System.Net.Http.HttpBaseStream.set_Position which means that there was an attempt to change the stream position pointer but the stream doesn't allow it (the exception message is not clear though).

I'm pretty sure the culprit is here. I'll submit a PR with fix to the FluentHttpClient project.

Edit: I raised an issue and provided code snippet to reproduce the problem. I also submitted a PR. I'll publish a new beta package when we get a hotfix for FluentHttpClient.

[Jericho]

My PR was accepted and a hotfix for FluentHttpClient was published earlier today. So I was able to publish an updated beta package to my NuGet feed: ZoomNet.0.77.0-pipeline-when-do0024.nupkg.

[pvgritsenko-ansible]

Hmm, 'DownloadFileAsync' method does not throw 'NotSupportedException', but it works wrong now. It looks like it tries to bufferize the file content again (since the method performing time depends on file size and it consumes the RAM memory before I start to read the stream.) And finally when I try to read the stream content, it returns nothing in any cases. It looks like the stream rewind to the end. I tried to call stream.Seek(0, SeekOrigin.Begin) after for the result stream but it is throws 'NotSupportedException'.

[Jericho]

Took me quite a while to figure it out: we have an error handler that inspects all responses from the Zoom API to determine if the call was successful. This handler assumes the content of the response contains JSON, parses this JSON into a document, looks for the presence of a few well-known properties in this parsed document and throws an exception with meaningful details when appropriate.

This concept works well in all "normal" scenarios where the API returns JSON but the current scenario that we are dealing with is not a typical scenario: the response contains a file rather than JSON. Not only does the response not contain JSON, but also we are streaming the content of the response therefore we must avoid reading the content of response.

Long story short: I modified how the request is dispatched in "DownloadFileAsync" to make sure our custom error handler is turned off.

Uploaded a new beta package called ZoomNet.0.77.0-pipeline-when-do0026.

[pvgritsenko-ansible]

Ah, I see. Good news. I've tested your last package version and it works well. And the method behaviour corresponds to the expectations (without buferring). So it looks ready to publish.

[Jericho]

Excellent. Thank you for confirming. I will publish the next version momentarily.

[Jericho]

Note to self: here's the snippet I used to debug this scenario

// Files of various sizes are available for testing purposes.
// See this article: https://www.buildsometech.com/download-test-files/#Download_Test_Files_100GB_50GB_10GB_5GB_4GB_2GB_1GB_High-Speed
const string downloadUrl = "https://proof.ovh.net/files/1Mb.dat";

var downloadStream = await client.CloudRecordings.DownloadFileAsync(downloadUrl, cancellationToken).ConfigureAwait(false);

using (Stream streamToWriteTo = File.Open(@"C:\temp\downloadedFile.dat", FileMode.Create))
{
    await downloadStream.CopyToAsync(streamToWriteTo);
}