Skip to content
This repository was archived by the owner on Jul 30, 2024. It is now read-only.

Commit 61171e5

Browse files
authored
[Statistics] Record FTP exceptions to logs (#463)
This also tweaks the retry pattern inside of `FtpDownloadStream`. Previously, `FtpDownloadStream` would dispose itself if a download failed. It will now disposes just the stream that failed to download.
1 parent 67bd2d1 commit 61171e5

2 files changed

Lines changed: 14 additions & 29 deletions

File tree

src/Stats.CollectAzureCdnLogs/Ftp/FtpDownloadStream.cs

Lines changed: 13 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ internal sealed class FtpDownloadStream
1616
private readonly Uri _uri;
1717
private Stream _stream;
1818
private int _totalDone;
19-
private bool _disposing;
2019

2120
public Exception CaughtException { get; private set; }
2221

@@ -31,12 +30,11 @@ public override async Task<int> ReadAsync(byte[] buffer, int offset, int count,
3130
var attempts = 0;
3231
while (attempts++ < 5)
3332
{
34-
var uriString = _uri.ToString();
3533
if (_stream == null)
3634
{
3735
_stream = await _client.StartOrResumeFtpDownload(_uri, _totalDone);
3836

39-
_client.Logger.LogInformation("Finishing download from '{FtpBlobUri}'", uriString);
37+
_client.Logger.LogInformation("Finishing download from '{FtpBlobUri}'", _uri);
4038
}
4139
try
4240
{
@@ -51,11 +49,11 @@ public override async Task<int> ReadAsync(byte[] buffer, int offset, int count,
5149
{
5250
CaughtException = ex;
5351

54-
_client.Logger.LogError("Failed to download file.", ex);
52+
_client.Logger.LogError(0, ex, "Failed to download file after {Attempts} attempts", attempts);
5553

56-
// Close ftp resources if possible.
57-
// Set instances to null to force restart.
58-
Close();
54+
// Close ftp resources and set instance to null to restart the download.
55+
_stream?.Dispose();
56+
_stream = null;
5957
}
6058
}
6159
return 0;
@@ -86,34 +84,22 @@ public override int Read(byte[] buffer, int offset, int count)
8684
{
8785
CaughtException = ex;
8886

89-
// Close ftp resources if possible. Set instances to null to force restart.
90-
Close();
87+
_client.Logger.LogError(0, ex, "Failed to download file after {Attempts}", attempts);
88+
89+
// Close ftp resources and set instance to null to restart the download.
90+
_stream?.Dispose();
91+
_stream = null;
9192
}
9293
}
9394
return 0;
9495
}
9596

96-
public override void Close()
97+
protected override void Dispose(bool disposing)
9798
{
98-
if (_disposing)
99-
{
100-
return;
101-
}
102-
103-
_disposing = true;
104-
105-
if (_stream != null)
99+
if (disposing)
106100
{
107-
try
108-
{
109-
_stream.Close();
110-
}
111-
catch
112-
{
113-
// No action required
114-
}
101+
_stream?.Dispose();
115102
}
116-
_stream = null;
117103
}
118104

119105
public override void Flush()

src/Stats.CollectAzureCdnLogs/Ftp/FtpRawLogClient.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,12 +186,11 @@ internal async Task<Stream> StartOrResumeFtpDownload(Uri uri, int contentOffset
186186
{
187187
if (contentOffset == 0)
188188
{
189-
Logger.LogInformation("Beginning download from '{FtpBlobUri}'.", uri.ToString());
190189
Logger.LogInformation("Downloading file '{FtpBlobUri}'.", uri);
191190
}
192191
else
193192
{
194-
Logger.LogInformation("Resuming download from '{FtpBlobUri}' at content offset {ContentOffset}.", uri.ToString(), contentOffset);
193+
Logger.LogInformation("Resuming download from '{FtpBlobUri}' at content offset {ContentOffset}.", uri, contentOffset);
195194
}
196195

197196
var request = CreateRequest(uri);

0 commit comments

Comments
 (0)