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

Issue: The DownloadFileAsync method will throw an OutOfMemoryException if the file is too large #342

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 36 additions & 10 deletions Source/ZoomNet/Resources/CloudRecordings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
using System.IO;
using System.Linq;
using System.Net.Http;
using System.Net.Http.Headers;
using System.Text.Json.Nodes;
using System.Threading;
using System.Threading.Tasks;
using ZoomNet.Models;
using ZoomNet.Utilities;

namespace ZoomNet.Resources
{
Expand Down Expand Up @@ -356,20 +358,44 @@ public Task RejectRegistrantsAsync(long meetingId, IEnumerable<string> registran
return UpdateRegistrantsStatusAsync(meetingId, registrantIds, "deny", cancellationToken);
}

/// <summary>
/// Download the recording file.
/// </summary>
/// <param name="downloadUrl">The URL of the recording file to download.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>
/// The <see cref="Stream"/> containing the file.
/// </returns>
public Task<Stream> DownloadFileAsync(string downloadUrl, CancellationToken cancellationToken = default)
/// <inheritdoc/>
public async Task<Stream> DownloadFileAsync(string downloadUrl, CancellationToken cancellationToken = default)
{
return _client
/*
* PLEASE NOTE:
*
* The HttpRequestMessage in this method is dispatched with its completion option set to "ResponseHeadersRead".
* This ensures the content of the response is streamed rather than buffered in memory.
* This is important in cases where the downloaded file is quite large.
* In this scenario, we don't want the entirety of the file to be buffered in a MemoryStream because
* it could lead to "out of memory" exceptions if the file is large enough.
* See https://github.com/Jericho/ZoomNet/pull/342 for a discussion on this topic.
*
* Forthermore, as of this writing, the FluentHttp library does not allow us to stream the content of responses
* which means that the code in this method cannot be simplified like so:
return _client
.GetAsync(downloadUrl)
.WithCancellationToken(cancellationToken)
.AsStream();
*
* The downside of not using the FluentHttp library to dispatch the request is that we lose automatic retries,
* error handling, logging, etc.
*/

using (var request = new HttpRequestMessage(HttpMethod.Get, downloadUrl))
Jericho marked this conversation as resolved.
Show resolved Hide resolved
{
var tokenHandler = _client.Filters.OfType<ITokenHandler>().SingleOrDefault();
if (tokenHandler != null)
{
request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", tokenHandler.Token);
}

var response = await _client.BaseClient.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, cancellationToken).ConfigureAwait(false);

response.EnsureSuccessStatusCode();

return await response.Content.ReadAsStreamAsync();
}
}

private Task UpdateRegistrantsStatusAsync(long meetingId, IEnumerable<string> registrantIds, string status, CancellationToken cancellationToken = default)
Expand Down