diff --git a/src/ByteSync.Client/Interfaces/Controls/Communications/IAdaptiveUploadController.cs b/src/ByteSync.Client/Interfaces/Controls/Communications/IAdaptiveUploadController.cs index a7572602a..58453631d 100644 --- a/src/ByteSync.Client/Interfaces/Controls/Communications/IAdaptiveUploadController.cs +++ b/src/ByteSync.Client/Interfaces/Controls/Communications/IAdaptiveUploadController.cs @@ -1,3 +1,5 @@ +using ByteSync.Common.Business.Communications.Transfers; + namespace ByteSync.Interfaces.Controls.Communications; public interface IAdaptiveUploadController @@ -8,6 +10,5 @@ public interface IAdaptiveUploadController int GetNextChunkSizeBytes(); - void RecordUploadResult(TimeSpan elapsed, bool isSuccess, int partNumber, int? statusCode = null, - Exception? exception = null, string? fileId = null, long actualBytes = -1); + void RecordUploadResult(UploadResult uploadResult); } diff --git a/src/ByteSync.Client/Services/Communications/Transfers/Strategies/AzureBlobStorageUploadStrategy.cs b/src/ByteSync.Client/Services/Communications/Transfers/Strategies/AzureBlobStorageUploadStrategy.cs index a39b0a9cf..42b62e54b 100644 --- a/src/ByteSync.Client/Services/Communications/Transfers/Strategies/AzureBlobStorageUploadStrategy.cs +++ b/src/ByteSync.Client/Services/Communications/Transfers/Strategies/AzureBlobStorageUploadStrategy.cs @@ -27,7 +27,7 @@ public async Task UploadAsync(FileUploaderSlice slice, FileS var blob = new BlobClient(new Uri(storageLocation.Url), options); var response = await blob.UploadAsync(slice.MemoryStream, cancellationToken); - _logger.LogDebug("UploadAvailableSlice: slice {number} is uploaded", slice.PartNumber); + _logger.LogDebug("UploadAvailableSlice: slice {PartNumber} is uploaded", slice.PartNumber); var rawResponse = response.GetRawResponse(); @@ -45,10 +45,15 @@ public async Task UploadAsync(FileUploaderSlice slice, FileS ); } } + catch (OperationCanceledException ex) + { + _logger.LogWarning(ex, "Upload of slice {PartNumber} was canceled or timed out", slice.PartNumber); + return UploadFailureClassifier.Classify(ex, cancellationToken); + } catch (Exception ex) { - _logger.LogError(ex, "Failed to upload slice {number}", slice.PartNumber); - return UploadFileResponse.Failure(500, ex); + _logger.LogError(ex, "Failed to upload slice {PartNumber}", slice.PartNumber); + return UploadFailureClassifier.Classify(ex, cancellationToken); } } } \ No newline at end of file diff --git a/src/ByteSync.Client/Services/Communications/Transfers/Strategies/CloudflareR2UploadStrategy.cs b/src/ByteSync.Client/Services/Communications/Transfers/Strategies/CloudflareR2UploadStrategy.cs index ed188739e..a22df878a 100644 --- a/src/ByteSync.Client/Services/Communications/Transfers/Strategies/CloudflareR2UploadStrategy.cs +++ b/src/ByteSync.Client/Services/Communications/Transfers/Strategies/CloudflareR2UploadStrategy.cs @@ -76,10 +76,15 @@ public async Task UploadAsync(FileUploaderSlice slice, FileS ); } } + catch (OperationCanceledException ex) + { + _logger.LogWarning(ex, "Upload of slice {PartNumber} was canceled or timed out", slice.PartNumber); + return UploadFailureClassifier.Classify(ex, cancellationToken); + } catch (Exception ex) { - _logger.LogError(ex, "Failed to upload slice {number}", slice.PartNumber); - return UploadFileResponse.Failure(500, ex); + _logger.LogError(ex, "Failed to upload slice {PartNumber}", slice.PartNumber); + return UploadFailureClassifier.Classify(ex, cancellationToken); } } } diff --git a/src/ByteSync.Client/Services/Communications/Transfers/Strategies/UploadFailureClassifier.cs b/src/ByteSync.Client/Services/Communications/Transfers/Strategies/UploadFailureClassifier.cs new file mode 100644 index 000000000..37418add8 --- /dev/null +++ b/src/ByteSync.Client/Services/Communications/Transfers/Strategies/UploadFailureClassifier.cs @@ -0,0 +1,23 @@ +using System; +using System.Threading; +using ByteSync.Common.Business.Communications.Transfers; + +namespace ByteSync.Services.Communications.Transfers.Strategies; + +public static class UploadFailureClassifier +{ + public static UploadFileResponse Classify(Exception exception, CancellationToken cancellationToken) + { + if (exception is OperationCanceledException && cancellationToken.IsCancellationRequested) + { + return UploadFileResponse.ClientCancellation(exception); + } + + if (exception is OperationCanceledException) + { + return UploadFileResponse.ClientTimeout(exception); + } + + return UploadFileResponse.Failure(500, exception); + } +} diff --git a/src/ByteSync.Client/Services/Communications/Transfers/Uploading/AdaptiveUploadController.cs b/src/ByteSync.Client/Services/Communications/Transfers/Uploading/AdaptiveUploadController.cs index a020cdf02..07cbb59aa 100644 --- a/src/ByteSync.Client/Services/Communications/Transfers/Uploading/AdaptiveUploadController.cs +++ b/src/ByteSync.Client/Services/Communications/Transfers/Uploading/AdaptiveUploadController.cs @@ -1,5 +1,6 @@ using System.Reactive.Linq; using ByteSync.Business.Sessions; +using ByteSync.Common.Business.Communications.Transfers; using ByteSync.Interfaces.Controls.Communications; using ByteSync.Interfaces.Services.Sessions; @@ -81,14 +82,18 @@ public int GetNextChunkSizeBytes() } } - public void RecordUploadResult(TimeSpan elapsed, bool isSuccess, int partNumber, int? statusCode = null, - Exception? exception = null, string? fileId = null, long actualBytes = -1) + public void RecordUploadResult(UploadResult uploadResult) { lock (_syncRoot) { - EnqueueSample(elapsed, isSuccess, actualBytes); + if (IsClientSideFailure(uploadResult.FailureKind)) + { + return; + } + + EnqueueSample(uploadResult.Elapsed, uploadResult.IsSuccess, uploadResult.ActualBytes); - if (HandleBandwidthReset(isSuccess, statusCode)) + if (HandleBandwidthReset(uploadResult.IsSuccess, uploadResult.StatusCode)) { return; } @@ -102,18 +107,18 @@ public void RecordUploadResult(TimeSpan elapsed, bool isSuccess, int partNumber, _logger.LogDebug( "Adaptive: file {FileId} maxElapsed={MaxElapsedMs} ms, window={Window}, parallelism={Parallelism}, chunkSize={ChunkKb} KB", - fileId ?? "-", + uploadResult.FileId ?? "-", maxElapsed.TotalMilliseconds, _windowSize, _currentParallelism, Math.Round(_currentChunkSizeBytes / 1024d)); - if (TryHandleDownscale(maxElapsed, fileId)) + if (TryHandleDownscale(maxElapsed, uploadResult.FileId)) { return; } - TryHandleUpscale(fileId); + TryHandleUpscale(uploadResult.FileId); } } @@ -146,6 +151,11 @@ private void EnqueueSample(TimeSpan elapsed, bool isSuccess, long actualBytes) } } + private static bool IsClientSideFailure(UploadFailureKind failureKind) + { + return failureKind is UploadFailureKind.ClientCancellation or UploadFailureKind.ClientTimeout; + } + private bool HandleBandwidthReset(bool isSuccess, int? statusCode) { if (!isSuccess && statusCode != null) @@ -278,7 +288,7 @@ private void TryHandleUpscale(string? fileId) } } - private double GetUpscaleMultiplier(TimeSpan maxElapsedEligible) + private static double GetUpscaleMultiplier(TimeSpan maxElapsedEligible) { if (maxElapsedEligible < TimeSpan.FromSeconds(1)) { diff --git a/src/ByteSync.Client/Services/Communications/Transfers/Uploading/FileUploadWorker.cs b/src/ByteSync.Client/Services/Communications/Transfers/Uploading/FileUploadWorker.cs index 236e0087f..07483dbdc 100644 --- a/src/ByteSync.Client/Services/Communications/Transfers/Uploading/FileUploadWorker.cs +++ b/src/ByteSync.Client/Services/Communications/Transfers/Uploading/FileUploadWorker.cs @@ -1,3 +1,4 @@ +using System.Diagnostics; using System.Threading; using System.Threading.Channels; using Autofac.Features.Indexed; @@ -23,10 +24,15 @@ public class FileUploadWorker : IFileUploadWorker private readonly SemaphoreSlim _semaphoreSlim; private readonly IAdaptiveUploadController _adaptiveUploadController; private readonly SemaphoreSlim _uploadSlots; - + private CancellationTokenSource CancellationTokenSource { get; } + private static int _workerTaskCounter; - + + private const int AttemptTimeoutFloorSeconds = 60; + private const int AttemptTimeoutCeilingSeconds = 120; + private const int SecondsPerMegabyteHeuristic = 3; + public FileUploadWorker( IPolicyFactory policyFactory, IFileTransferApiClient fileTransferApiClient, @@ -51,7 +57,7 @@ public FileUploadWorker( _uploadSlots = uploadSlots; CancellationTokenSource = new CancellationTokenSource(); } - + // Backward-compatible overload for tests and callers not providing uploadSlots public FileUploadWorker( IPolicyFactory policyFactory, @@ -76,7 +82,7 @@ public FileUploadWorker( new SemaphoreSlim(Math.Min(Math.Max(1, adaptiveUploadController.CurrentParallelism), 4), 4)) { } - + public async Task UploadAvailableSlicesAdaptiveAsync(Channel availableSlices, UploadProgressState progressState) { var workerId = Interlocked.Increment(ref _workerTaskCounter); @@ -86,28 +92,29 @@ public async Task UploadAvailableSlicesAdaptiveAsync(Channel { continue; } - + try { - var sliceStart = System.Diagnostics.Stopwatch.StartNew(); - + var sliceStart = Stopwatch.StartNew(); + await IncrementConcurrentAsync(progressState); var policy = _policyFactory.BuildFileUploadPolicy(); var attempt = 0; - + var response = await policy.ExecuteAsync(async () => { attempt++; + return await ExecuteUploadAttemptAsync(slice, workerId, attempt, CancellationTokenSource.Token); }); - + EnsureSuccessOrThrow(response); var fileName = _sharedFileDefinition.GetFileName(slice.PartNumber); - var assertSw = System.Diagnostics.Stopwatch.StartNew(); + var assertSw = Stopwatch.StartNew(); _logger.LogDebug("UploadAvailableSlice: worker {WorkerId} start asserting slice {Number} for {FileName}", workerId, slice.PartNumber, fileName); - + var transferParameters = new TransferParameters { SessionId = _sharedFileDefinition.SessionId, @@ -115,52 +122,56 @@ public async Task UploadAvailableSlicesAdaptiveAsync(Channel PartNumber = slice.PartNumber, PartSizeInBytes = slice.MemoryStream.Length }; - + await AssertSliceUploadedAsync(policy, transferParameters, workerId, slice.PartNumber, fileName, assertSw); assertSw.Stop(); - _logger.LogDebug("UploadAvailableSlice: worker {WorkerId} finished asserting slice {Number} for {FileName} in {ElapsedMs} ms", + _logger.LogDebug( + "UploadAvailableSlice: worker {WorkerId} finished asserting slice {Number} for {FileName} in {ElapsedMs} ms", workerId, slice.PartNumber, fileName, assertSw.ElapsedMilliseconds); - + // Success path bookkeeping await UpdateProgressOnSuccessAsync(progressState, slice, sliceStart); } catch (Exception ex) { await HandleUploadExceptionAsync(progressState, ex, workerId); + return; } finally { DisposeSlice(slice); await DecrementConcurrentAsync(progressState); + // No final release here: attempts handled slot release per attempt } } - + await CompleteIfFinishedAsync(progressState); } - - private async Task ExecuteUploadAttemptAsync(FileUploaderSlice slice, int workerId, int attempt, CancellationToken globalToken) + + private async Task ExecuteUploadAttemptAsync(FileUploaderSlice slice, int workerId, int attempt, + CancellationToken globalToken) { var attemptStart = DateTime.UtcNow; var timeoutSec = ComputeAttemptTimeoutSeconds(slice); using var attemptCts = CancellationTokenSource.CreateLinkedTokenSource(globalToken); attemptCts.CancelAfter(TimeSpan.FromSeconds(timeoutSec)); - + var beforeWait = _uploadSlots.CurrentCount; _logger.LogDebug("UploadAvailableSlice: worker {WorkerId} waiting for upload slot (available {Available})", workerId, beforeWait); - + var acquired = false; try { await _uploadSlots.WaitAsync(attemptCts.Token); acquired = true; - + var afterWait = _uploadSlots.CurrentCount; _logger.LogDebug("UploadAvailableSlice: worker {WorkerId} acquired upload slot (available now {Available}), attempt {Attempt}", workerId, afterWait, attempt); - + var uploadTask = DoUpload(slice, workerId, attemptCts.Token); var heartbeat = TimeSpan.FromSeconds(30); while (!uploadTask.IsCompleted) @@ -170,36 +181,64 @@ private async Task ExecuteUploadAttemptAsync(FileUploaderSli { break; } - + var fileNameHb = _sharedFileDefinition.GetFileName(slice.PartNumber); _logger.LogDebug( "UploadAvailableSlice: worker {WorkerId} uploading slice {Number} for {FileName}... attempt {Attempt}, elapsed {ElapsedMs} ms", workerId, slice.PartNumber, fileNameHb, attempt, (DateTime.UtcNow - attemptStart).TotalMilliseconds); - + if (attemptCts.IsCancellationRequested) { _logger.LogWarning( "UploadAvailableSlice: worker {WorkerId} upload attempt {Attempt} timed out after ~{TimeoutSec}s; waiting for cancellation...", workerId, attempt, timeoutSec); + break; } } - + var attemptResponse = await uploadTask; var elapsed = DateTime.UtcNow - attemptStart; - _adaptiveUploadController.RecordUploadResult(elapsed, attemptResponse.IsSuccess, slice.PartNumber, attemptResponse.StatusCode, null, _sharedFileDefinition.Id, slice.MemoryStream.Length); + var refinedKind = RefineFailureKind(attemptResponse.FailureKind, attemptCts, globalToken); + _adaptiveUploadController.RecordUploadResult(new UploadResult( + elapsed, + attemptResponse.IsSuccess, + slice.PartNumber, + attemptResponse.StatusCode, + FileId: _sharedFileDefinition.Id, + ActualBytes: slice.MemoryStream.Length, + FailureKind: refinedKind)); + return attemptResponse; } catch (OperationCanceledException oce) { var elapsed = DateTime.UtcNow - attemptStart; - _adaptiveUploadController.RecordUploadResult(elapsed, false, slice.PartNumber, null, oce, _sharedFileDefinition.Id, slice.MemoryStream.Length); + var kind = DetermineCancellationKind(attemptCts, globalToken); + _adaptiveUploadController.RecordUploadResult(new UploadResult( + elapsed, + false, + slice.PartNumber, + Exception: oce, + FileId: _sharedFileDefinition.Id, + ActualBytes: slice.MemoryStream.Length, + FailureKind: kind)); + throw new TaskCanceledException("Upload attempt canceled during slot wait or upload.", oce); } catch (Exception ex) { var elapsed = DateTime.UtcNow - attemptStart; - _adaptiveUploadController.RecordUploadResult(elapsed, false, slice.PartNumber, 500, ex, _sharedFileDefinition.Id, slice.MemoryStream.Length); + _adaptiveUploadController.RecordUploadResult(new UploadResult( + elapsed, + false, + slice.PartNumber, + 500, + ex, + _sharedFileDefinition.Id, + slice.MemoryStream.Length, + UploadFailureKind.ServerError)); + throw; } finally @@ -217,7 +256,9 @@ private async Task ExecuteUploadAttemptAsync(FileUploaderSli } else { - _logger.LogDebug("UploadAvailableSlice: worker {WorkerId} did not acquire upload slot (canceled before acquire) for attempt {Attempt}", workerId, attempt); + _logger.LogDebug( + "UploadAvailableSlice: worker {WorkerId} did not acquire upload slot (canceled before acquire) for attempt {Attempt}", + workerId, attempt); } } catch (Exception ex) @@ -227,28 +268,61 @@ private async Task ExecuteUploadAttemptAsync(FileUploaderSli } } } - + private static int ComputeAttemptTimeoutSeconds(FileUploaderSlice slice) { - var sizeMb = Math.Max(1, (int)Math.Ceiling((slice.MemoryStream.Length) / (1024d * 1024d))); - var timeoutSec = Math.Clamp(3 * sizeMb, 30, 90); + return ComputeAttemptTimeoutSeconds(slice.MemoryStream.Length); + } + + private static int ComputeAttemptTimeoutSeconds(long sliceLengthBytes) + { + var sizeMb = Math.Max(1, (int)Math.Ceiling(sliceLengthBytes / (1024d * 1024d))); + var timeoutSec = Math.Clamp(SecondsPerMegabyteHeuristic * sizeMb, AttemptTimeoutFloorSeconds, AttemptTimeoutCeilingSeconds); + return timeoutSec; } - + + private static UploadFailureKind RefineFailureKind(UploadFailureKind kind, CancellationTokenSource attemptCts, + CancellationToken globalToken) + { + if (kind != UploadFailureKind.ClientCancellation) + { + return kind; + } + + return DetermineCancellationKind(attemptCts, globalToken); + } + + private static UploadFailureKind DetermineCancellationKind(CancellationTokenSource attemptCts, CancellationToken globalToken) + { + if (globalToken.IsCancellationRequested) + { + return UploadFailureKind.ClientCancellation; + } + + if (attemptCts.IsCancellationRequested) + { + return UploadFailureKind.ClientTimeout; + } + + return UploadFailureKind.ClientCancellation; + } + private async Task AssertSliceUploadedAsync( AsyncRetryPolicy policy, TransferParameters transferParameters, int workerId, int partNumber, string fileName, - System.Diagnostics.Stopwatch assertSw) + Stopwatch assertSw) { var assertTask = policy.ExecuteAsync(async () => { await _fileTransferApiClient.AssertFilePartIsUploaded(transferParameters); + return UploadFileResponse.Success(200); }); - + while (!assertTask.IsCompleted) { var completed = await Task.WhenAny(assertTask, Task.Delay(TimeSpan.FromSeconds(30))); @@ -259,10 +333,10 @@ private async Task AssertSliceUploadedAsync( workerId, partNumber, fileName, assertSw.ElapsedMilliseconds); } } - + await assertTask; } - + private async Task IncrementConcurrentAsync(UploadProgressState progressState) { await _semaphoreSlim.WaitAsync(); @@ -279,7 +353,7 @@ private async Task IncrementConcurrentAsync(UploadProgressState progressState) _semaphoreSlim.Release(); } } - + private async Task DecrementConcurrentAsync(UploadProgressState progressState) { await _semaphoreSlim.WaitAsync(); @@ -295,16 +369,17 @@ private async Task DecrementConcurrentAsync(UploadProgressState progressState) _semaphoreSlim.Release(); } } - + private static void EnsureSuccessOrThrow(UploadFileResponse? response) { if (response == null || !response.IsSuccess) { - throw new Exception($"UploadAvailableSlice: unable to get upload url. Status: {response?.StatusCode}, Error: {response?.ErrorMessage}"); + throw new Exception( + $"UploadAvailableSlice: upload attempt failed. Status: {response?.StatusCode}, Error: {response?.ErrorMessage}"); } } - - private async Task UpdateProgressOnSuccessAsync(UploadProgressState progressState, FileUploaderSlice slice, System.Diagnostics.Stopwatch? sliceStart) + + private async Task UpdateProgressOnSuccessAsync(UploadProgressState progressState, FileUploaderSlice slice, Stopwatch? sliceStart) { await _semaphoreSlim.WaitAsync(); try @@ -323,11 +398,11 @@ private async Task UpdateProgressOnSuccessAsync(UploadProgressState progressStat _semaphoreSlim.Release(); } } - + private async Task HandleUploadExceptionAsync(UploadProgressState progressState, Exception ex, int workerId) { _logger.LogError(ex, "UploadAvailableSlice: worker {WorkerId} error", workerId); - + await _semaphoreSlim.WaitAsync(); try { @@ -337,10 +412,10 @@ private async Task HandleUploadExceptionAsync(UploadProgressState progressState, { _semaphoreSlim.Release(); } - + _exceptionOccurred.Set(); } - + private void DisposeSlice(FileUploaderSlice slice) { try @@ -352,7 +427,7 @@ private void DisposeSlice(FileUploaderSlice slice) _logger.LogWarning(ex, "Error disposing slice {Number} memory stream", slice.PartNumber); } } - + private async Task CompleteIfFinishedAsync(UploadProgressState progressState) { await _semaphoreSlim.WaitAsync(); @@ -370,7 +445,7 @@ private async Task CompleteIfFinishedAsync(UploadProgressState progressState) _semaphoreSlim.Release(); } } - + private async Task DoUpload(FileUploaderSlice slice, int workerId, CancellationToken cancellationToken) { try @@ -381,28 +456,31 @@ private async Task DoUpload(FileUploaderSlice slice, int wor SharedFileDefinition = _sharedFileDefinition, PartNumber = slice.PartNumber }; - + var uploadLocation = await _fileTransferApiClient.GetUploadFileStorageLocation(transferParameters); var lengthKbRounded = (long)Math.Round((slice.MemoryStream.Length) / 1024d); var fileName = _sharedFileDefinition.GetFileName(slice.PartNumber); _logger.LogDebug("UploadAvailableSlice: worker {WorkerId} start uploading slice {Number} for {FileName} ({LengthKb} KB)", workerId, slice.PartNumber, fileName, lengthKbRounded); - + var uploadStrategy = _strategies[uploadLocation.StorageProvider]; - var sw = System.Diagnostics.Stopwatch.StartNew(); + var sw = Stopwatch.StartNew(); var response = await uploadStrategy.UploadAsync(slice, uploadLocation, cancellationToken); sw.Stop(); - _logger.LogDebug("UploadAvailableSlice: worker {WorkerId} finished uploading slice {Number} for {FileName} ({LengthKb} KB) in {ElapsedMs} ms (status {Status})", + _logger.LogDebug( + "UploadAvailableSlice: worker {WorkerId} finished uploading slice {Number} for {FileName} ({LengthKb} KB) in {ElapsedMs} ms (status {Status})", workerId, slice.PartNumber, fileName, lengthKbRounded, sw.ElapsedMilliseconds, response.StatusCode); - + return response; } catch (Exception ex) { var fileName = _sharedFileDefinition.GetFileName(slice.PartNumber); - _logger.LogError(ex, "Error while uploading slice {Number} for {FileName} (worker {WorkerId}), sharedFileDefinitionId:{sharedFileDefinitionId} ", + _logger.LogError(ex, + "Error while uploading slice {Number} for {FileName} (worker {WorkerId}), sharedFileDefinitionId:{sharedFileDefinitionId} ", slice.PartNumber, fileName, workerId, _sharedFileDefinition.Id); + throw; } } -} +} \ No newline at end of file diff --git a/src/ByteSync.Common/Business/Communications/Transfers/UploadFailureKind.cs b/src/ByteSync.Common/Business/Communications/Transfers/UploadFailureKind.cs new file mode 100644 index 000000000..11db98f12 --- /dev/null +++ b/src/ByteSync.Common/Business/Communications/Transfers/UploadFailureKind.cs @@ -0,0 +1,9 @@ +namespace ByteSync.Common.Business.Communications.Transfers; + +public enum UploadFailureKind +{ + None = 0, + ServerError = 1, + ClientCancellation = 2, + ClientTimeout = 3, +} diff --git a/src/ByteSync.Common/Business/Communications/Transfers/UploadFileResponse.cs b/src/ByteSync.Common/Business/Communications/Transfers/UploadFileResponse.cs index 5ea210421..c50b526f6 100644 --- a/src/ByteSync.Common/Business/Communications/Transfers/UploadFileResponse.cs +++ b/src/ByteSync.Common/Business/Communications/Transfers/UploadFileResponse.cs @@ -8,6 +8,7 @@ public class UploadFileResponse public int StatusCode { get; set; } public string? ErrorMessage { get; set; } public Exception? Exception { get; set; } + public UploadFailureKind FailureKind { get; set; } = UploadFailureKind.None; public static UploadFileResponse Success(int statusCode) { @@ -15,6 +16,7 @@ public static UploadFileResponse Success(int statusCode) { IsSuccess = true, StatusCode = statusCode, + FailureKind = UploadFailureKind.None, }; } @@ -25,6 +27,7 @@ public static UploadFileResponse Failure(int statusCode, string errorMessage) IsSuccess = false, StatusCode = statusCode, ErrorMessage = errorMessage, + FailureKind = UploadFailureKind.ServerError, }; } @@ -36,6 +39,31 @@ public static UploadFileResponse Failure(int statusCode, Exception exception) StatusCode = statusCode, ErrorMessage = exception.Message, Exception = exception, + FailureKind = UploadFailureKind.ServerError, + }; + } + + public static UploadFileResponse ClientCancellation(Exception exception) + { + return new UploadFileResponse + { + IsSuccess = false, + StatusCode = 0, + ErrorMessage = exception.Message, + Exception = exception, + FailureKind = UploadFailureKind.ClientCancellation, + }; + } + + public static UploadFileResponse ClientTimeout(Exception exception) + { + return new UploadFileResponse + { + IsSuccess = false, + StatusCode = 0, + ErrorMessage = exception.Message, + Exception = exception, + FailureKind = UploadFailureKind.ClientTimeout, }; } } diff --git a/src/ByteSync.Common/Business/Communications/Transfers/UploadResult.cs b/src/ByteSync.Common/Business/Communications/Transfers/UploadResult.cs new file mode 100644 index 000000000..a9c367e2e --- /dev/null +++ b/src/ByteSync.Common/Business/Communications/Transfers/UploadResult.cs @@ -0,0 +1,13 @@ +using System; + +namespace ByteSync.Common.Business.Communications.Transfers; + +public sealed record UploadResult( + TimeSpan Elapsed, + bool IsSuccess, + int PartNumber, + int? StatusCode = null, + Exception? Exception = null, + string? FileId = null, + long ActualBytes = -1, + UploadFailureKind FailureKind = UploadFailureKind.None); diff --git a/tests/ByteSync.Client.IntegrationTests/TestHelpers/FixedAdaptiveUploadController.cs b/tests/ByteSync.Client.IntegrationTests/TestHelpers/FixedAdaptiveUploadController.cs index c1d3d4d48..dd455ca4b 100644 --- a/tests/ByteSync.Client.IntegrationTests/TestHelpers/FixedAdaptiveUploadController.cs +++ b/tests/ByteSync.Client.IntegrationTests/TestHelpers/FixedAdaptiveUploadController.cs @@ -1,3 +1,4 @@ +using ByteSync.Common.Business.Communications.Transfers; using ByteSync.Interfaces.Controls.Communications; namespace ByteSync.Client.IntegrationTests.TestHelpers; @@ -18,7 +19,7 @@ public int GetNextChunkSizeBytes() return CurrentChunkSizeBytes; } - public void RecordUploadResult(TimeSpan elapsed, bool isSuccess, int partNumber, int? statusCode = null, Exception? exception = null, string? fileId = null, long actualBytes = -1) + public void RecordUploadResult(UploadResult uploadResult) { // no-op: fixed behavior for tests } diff --git a/tests/ByteSync.Client.UnitTests/Common/UploadFileResponseTests.cs b/tests/ByteSync.Client.UnitTests/Common/UploadFileResponseTests.cs new file mode 100644 index 000000000..a99b17acf --- /dev/null +++ b/tests/ByteSync.Client.UnitTests/Common/UploadFileResponseTests.cs @@ -0,0 +1,83 @@ +using ByteSync.Common.Business.Communications.Transfers; +using FluentAssertions; +using NUnit.Framework; + +namespace ByteSync.Client.UnitTests.Common; + +[TestFixture] +public class UploadFileResponseTests +{ + [Test] + public void Success_ShouldHaveNoFailureKind() + { + var response = UploadFileResponse.Success(204); + + response.IsSuccess.Should().BeTrue(); + response.StatusCode.Should().Be(204); + response.FailureKind.Should().Be(UploadFailureKind.None); + response.Exception.Should().BeNull(); + response.ErrorMessage.Should().BeNull(); + } + + [Test] + public void FailureWithMessage_ShouldBeServerError() + { + var response = UploadFileResponse.Failure(503, "Service unavailable"); + + response.IsSuccess.Should().BeFalse(); + response.StatusCode.Should().Be(503); + response.ErrorMessage.Should().Be("Service unavailable"); + response.FailureKind.Should().Be(UploadFailureKind.ServerError); + response.Exception.Should().BeNull(); + } + + [Test] + public void FailureWithException_ShouldBeServerError_AndCaptureException() + { + var ex = new InvalidOperationException("boom"); + + var response = UploadFileResponse.Failure(500, ex); + + response.IsSuccess.Should().BeFalse(); + response.StatusCode.Should().Be(500); + response.ErrorMessage.Should().Be("boom"); + response.Exception.Should().BeSameAs(ex); + response.FailureKind.Should().Be(UploadFailureKind.ServerError); + } + + [Test] + public void ClientCancellation_ShouldNotHaveServerStatusCode() + { + var ex = new OperationCanceledException("canceled"); + + var response = UploadFileResponse.ClientCancellation(ex); + + response.IsSuccess.Should().BeFalse(); + response.StatusCode.Should().Be(0); + response.ErrorMessage.Should().Be("canceled"); + response.Exception.Should().BeSameAs(ex); + response.FailureKind.Should().Be(UploadFailureKind.ClientCancellation); + } + + [Test] + public void ClientTimeout_ShouldBeDistinctFromCancellation() + { + var ex = new TaskCanceledException("timed out"); + + var response = UploadFileResponse.ClientTimeout(ex); + + response.IsSuccess.Should().BeFalse(); + response.StatusCode.Should().Be(0); + response.ErrorMessage.Should().Be("timed out"); + response.Exception.Should().BeSameAs(ex); + response.FailureKind.Should().Be(UploadFailureKind.ClientTimeout); + } + + [Test] + public void FailureKind_DefaultValueOnNewInstance_ShouldBeNone() + { + var response = new UploadFileResponse(); + + response.FailureKind.Should().Be(UploadFailureKind.None); + } +} diff --git a/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Strategies/CloudflareR2UploadStrategyTests.cs b/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Strategies/CloudflareR2UploadStrategyTests.cs new file mode 100644 index 000000000..e177d1934 --- /dev/null +++ b/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Strategies/CloudflareR2UploadStrategyTests.cs @@ -0,0 +1,189 @@ +using System.Net; +using ByteSync.Business.Communications.Transfers; +using ByteSync.Common.Business.Communications.Transfers; +using ByteSync.Common.Business.SharedFiles; +using ByteSync.Services.Communications.Transfers.Strategies; +using FluentAssertions; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Moq.Protected; +using NUnit.Framework; + +namespace ByteSync.Client.UnitTests.Services.Communications.Transfers.Strategies; + +[TestFixture] +public class CloudflareR2UploadStrategyTests +{ + private const string UploadUrl = "https://test-bucket.r2.cloudflarestorage.com/test/slice-1"; + + private static FileUploaderSlice CreateSlice(int partNumber = 1, int sizeBytes = 64) + { + var bytes = new byte[sizeBytes]; + Array.Fill(bytes, 0x42); + return new FileUploaderSlice(partNumber, new MemoryStream(bytes, writable: true)); + } + + private static FileStorageLocation CreateLocation() => new(UploadUrl, StorageProvider.CloudflareR2); + + private static (CloudflareR2UploadStrategy strategy, Mock handler) CreateStrategy() + { + var handler = new Mock(MockBehavior.Strict); + var factory = new Mock(); + factory + .Setup(f => f.CreateClient(It.IsAny())) + .Returns(() => new HttpClient(handler.Object, disposeHandler: false)); + + var strategy = new CloudflareR2UploadStrategy(NullLogger.Instance, factory.Object); + return (strategy, handler); + } + + private static void SetupHandler(Mock handler, HttpResponseMessage response) + { + handler.Protected() + .Setup>( + "SendAsync", + ItExpr.IsAny(), + ItExpr.IsAny()) + .ReturnsAsync(response); + } + + private static void SetupHandlerThrows(Mock handler, Exception exception) + { + handler.Protected() + .Setup>( + "SendAsync", + ItExpr.IsAny(), + ItExpr.IsAny()) + .ThrowsAsync(exception); + } + + [Test] + public async Task UploadAsync_On2xx_ShouldReturnSuccess() + { + var (strategy, handler) = CreateStrategy(); + SetupHandler(handler, new HttpResponseMessage(HttpStatusCode.OK)); + + var response = await strategy.UploadAsync(CreateSlice(), CreateLocation(), CancellationToken.None); + + response.IsSuccess.Should().BeTrue(); + response.StatusCode.Should().Be(200); + response.FailureKind.Should().Be(UploadFailureKind.None); + } + + [Test] + public async Task UploadAsync_On500_ShouldReturnServerFailure_WithRealStatusCode() + { + var (strategy, handler) = CreateStrategy(); + SetupHandler(handler, new HttpResponseMessage(HttpStatusCode.InternalServerError) + { + Content = new StringContent("boom") + }); + + var response = await strategy.UploadAsync(CreateSlice(), CreateLocation(), CancellationToken.None); + + response.IsSuccess.Should().BeFalse(); + response.StatusCode.Should().Be(500); + response.FailureKind.Should().Be(UploadFailureKind.ServerError); + } + + [Test] + public async Task UploadAsync_On429_ShouldReturnServerFailure_WithRealStatusCode() + { + var (strategy, handler) = CreateStrategy(); + SetupHandler(handler, new HttpResponseMessage((HttpStatusCode)429) + { + Content = new StringContent("rate limited") + }); + + var response = await strategy.UploadAsync(CreateSlice(), CreateLocation(), CancellationToken.None); + + response.IsSuccess.Should().BeFalse(); + response.StatusCode.Should().Be(429); + response.FailureKind.Should().Be(UploadFailureKind.ServerError); + } + + [Test] + public async Task UploadAsync_WhenCallerCancels_ShouldReturnClientCancellation() + { + var factory = new Mock(); + var hangingHandler = new HangingHttpMessageHandler(); + factory.Setup(f => f.CreateClient(It.IsAny())) + .Returns(() => new HttpClient(hangingHandler, disposeHandler: false)); + var strategy = new CloudflareR2UploadStrategy(NullLogger.Instance, factory.Object); + + using var cts = new CancellationTokenSource(); + cts.CancelAfter(TimeSpan.FromMilliseconds(50)); + + var response = await strategy.UploadAsync(CreateSlice(), CreateLocation(), cts.Token); + + response.IsSuccess.Should().BeFalse(); + response.FailureKind.Should().Be(UploadFailureKind.ClientCancellation); + response.StatusCode.Should().Be(0); + response.Exception.Should().BeAssignableTo(); + } + + private sealed class HangingHttpMessageHandler : HttpMessageHandler + { + protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + await Task.Delay(Timeout.Infinite, cancellationToken); + return new HttpResponseMessage(HttpStatusCode.OK); + } + } + + [Test] + public async Task UploadAsync_WhenHttpRequestExceptionThrown_ShouldReturnServerError500() + { + var (strategy, handler) = CreateStrategy(); + SetupHandlerThrows(handler, new HttpRequestException("socket reset")); + + var response = await strategy.UploadAsync(CreateSlice(), CreateLocation(), CancellationToken.None); + + response.IsSuccess.Should().BeFalse(); + response.FailureKind.Should().Be(UploadFailureKind.ServerError); + response.StatusCode.Should().Be(500); + response.Exception.Should().BeAssignableTo(); + } + + [Test] + public async Task UploadAsync_WhenIOExceptionThrown_ShouldReturnServerError500() + { + var (strategy, handler) = CreateStrategy(); + SetupHandlerThrows(handler, new IOException("broken pipe")); + + var response = await strategy.UploadAsync(CreateSlice(), CreateLocation(), CancellationToken.None); + + response.IsSuccess.Should().BeFalse(); + response.FailureKind.Should().Be(UploadFailureKind.ServerError); + response.StatusCode.Should().Be(500); + } + + [Test] + public async Task UploadAsync_WhenOperationCanceledThrown_WithCancelledToken_ShouldReturnClientCancellation() + { + var (strategy, handler) = CreateStrategy(); + using var cts = new CancellationTokenSource(); + cts.Cancel(); + SetupHandlerThrows(handler, new OperationCanceledException("cancel")); + + var response = await strategy.UploadAsync(CreateSlice(), CreateLocation(), cts.Token); + + response.IsSuccess.Should().BeFalse(); + response.FailureKind.Should().Be(UploadFailureKind.ClientCancellation); + response.StatusCode.Should().Be(0); + } + + [Test] + public async Task UploadAsync_WhenOperationCanceledThrown_WithLiveToken_ShouldReturnClientTimeout() + { + var (strategy, handler) = CreateStrategy(); + using var cts = new CancellationTokenSource(); + SetupHandlerThrows(handler, new OperationCanceledException("odd")); + + var response = await strategy.UploadAsync(CreateSlice(), CreateLocation(), cts.Token); + + response.IsSuccess.Should().BeFalse(); + response.FailureKind.Should().Be(UploadFailureKind.ClientTimeout); + response.StatusCode.Should().Be(0); + } +} diff --git a/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Strategies/UploadFailureClassifierTests.cs b/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Strategies/UploadFailureClassifierTests.cs new file mode 100644 index 000000000..0c8f8a9a4 --- /dev/null +++ b/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Strategies/UploadFailureClassifierTests.cs @@ -0,0 +1,95 @@ +using ByteSync.Common.Business.Communications.Transfers; +using ByteSync.Services.Communications.Transfers.Strategies; +using FluentAssertions; +using NUnit.Framework; + +namespace ByteSync.Client.UnitTests.Services.Communications.Transfers.Strategies; + +[TestFixture] +public class UploadFailureClassifierTests +{ + [Test] + public void Classify_OperationCanceledException_WithCancelledToken_ShouldReturnClientCancellation() + { + using var cts = new CancellationTokenSource(); + cts.Cancel(); + var ex = new OperationCanceledException("user cancel"); + + var response = UploadFailureClassifier.Classify(ex, cts.Token); + + response.IsSuccess.Should().BeFalse(); + response.FailureKind.Should().Be(UploadFailureKind.ClientCancellation); + response.Exception.Should().BeSameAs(ex); + } + + [Test] + public void Classify_TaskCanceledException_WithCancelledToken_ShouldReturnClientCancellation() + { + using var cts = new CancellationTokenSource(); + cts.Cancel(); + var ex = new TaskCanceledException("timed out"); + + var response = UploadFailureClassifier.Classify(ex, cts.Token); + + response.IsSuccess.Should().BeFalse(); + response.FailureKind.Should().Be(UploadFailureKind.ClientCancellation); + response.Exception.Should().BeSameAs(ex); + } + + [Test] + public void Classify_OperationCanceledException_WithNonCancelledToken_ShouldReturnClientTimeout() + { + using var cts = new CancellationTokenSource(); + var ex = new OperationCanceledException("odd"); + + var response = UploadFailureClassifier.Classify(ex, cts.Token); + + response.IsSuccess.Should().BeFalse(); + response.FailureKind.Should().Be(UploadFailureKind.ClientTimeout); + response.StatusCode.Should().Be(0); + response.Exception.Should().BeSameAs(ex); + } + + [Test] + public void Classify_TaskCanceledException_WithNonCancelledToken_ShouldReturnClientTimeout() + { + using var cts = new CancellationTokenSource(); + var ex = new TaskCanceledException("http timeout"); + + var response = UploadFailureClassifier.Classify(ex, cts.Token); + + response.IsSuccess.Should().BeFalse(); + response.FailureKind.Should().Be(UploadFailureKind.ClientTimeout); + response.StatusCode.Should().Be(0); + response.Exception.Should().BeSameAs(ex); + } + + [Test] + public void Classify_GenericException_ShouldReturnServerError500() + { + using var cts = new CancellationTokenSource(); + cts.Cancel(); + var ex = new InvalidOperationException("broken"); + + var response = UploadFailureClassifier.Classify(ex, cts.Token); + + response.IsSuccess.Should().BeFalse(); + response.StatusCode.Should().Be(500); + response.FailureKind.Should().Be(UploadFailureKind.ServerError); + response.Exception.Should().BeSameAs(ex); + } + + [Test] + public void Classify_HttpRequestException_ShouldReturnServerError500() + { + using var cts = new CancellationTokenSource(); + var ex = new HttpRequestException("network issue"); + + var response = UploadFailureClassifier.Classify(ex, cts.Token); + + response.IsSuccess.Should().BeFalse(); + response.StatusCode.Should().Be(500); + response.FailureKind.Should().Be(UploadFailureKind.ServerError); + response.Exception.Should().BeSameAs(ex); + } +} diff --git a/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Uploading/AdaptiveUploadControllerTests.cs b/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Uploading/AdaptiveUploadControllerTests.cs index 09ff98d42..e7e288257 100644 --- a/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Uploading/AdaptiveUploadControllerTests.cs +++ b/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Uploading/AdaptiveUploadControllerTests.cs @@ -1,5 +1,6 @@ using System.Reactive.Linq; using ByteSync.Business.Sessions; +using ByteSync.Common.Business.Communications.Transfers; using ByteSync.Common.Business.Sessions; using ByteSync.Interfaces.Services.Sessions; using ByteSync.Services.Communications.Transfers.Uploading; @@ -129,13 +130,110 @@ public void BandwidthErrors_ResetChunkSize() var parallelismBefore = _controller.CurrentParallelism; // Act - Record bandwidth-related failure (e.g., 429) - _controller.RecordUploadResult(TimeSpan.FromSeconds(1), isSuccess: false, partNumber: 1, statusCode: 429); + RecordUploadResult(_controller, TimeSpan.FromSeconds(1), isSuccess: false, partNumber: 1, statusCode: 429); // Assert _controller.CurrentChunkSizeBytes.Should().Be(500 * 1024); _controller.CurrentParallelism.Should().Be(parallelismBefore); } + [Test] + public void ClientCancellation_DoesNotResetChunkSize() + { + // Arrange - Inflate chunk somewhat first + FeedFastWindow(_controller); + var inflatedChunk = _controller.CurrentChunkSizeBytes; + inflatedChunk.Should().BeGreaterThan(500 * 1024); + var parallelismBefore = _controller.CurrentParallelism; + + // Act - Record a client cancellation (e.g., user pressed cancel) + RecordUploadResult( + _controller, + TimeSpan.FromSeconds(2), + isSuccess: false, + partNumber: 1, + statusCode: 0, + failureKind: UploadFailureKind.ClientCancellation); + + // Assert + _controller.CurrentChunkSizeBytes.Should().Be(inflatedChunk); + _controller.CurrentParallelism.Should().Be(parallelismBefore); + } + + [Test] + public void ClientTimeout_DoesNotResetChunkSize() + { + // Arrange - Inflate chunk somewhat first + FeedFastWindow(_controller); + var inflatedChunk = _controller.CurrentChunkSizeBytes; + inflatedChunk.Should().BeGreaterThan(500 * 1024); + var parallelismBefore = _controller.CurrentParallelism; + + // Act - Record a client-side timeout (our attempt CTS expired) + RecordUploadResult( + _controller, + TimeSpan.FromSeconds(60), + isSuccess: false, + partNumber: 1, + statusCode: 0, + failureKind: UploadFailureKind.ClientTimeout); + + // Assert + _controller.CurrentChunkSizeBytes.Should().Be(inflatedChunk); + _controller.CurrentParallelism.Should().Be(parallelismBefore); + } + + [Test] + public void ClientTimeout_DoesNotEnterAdaptiveWindow_AndDoesNotResetChunkSize() + { + // Arrange - Inflate chunk and make sure parallelism is just at min (=2) + var safety = 10; + while (_controller.CurrentChunkSizeBytes < 1024 * 1024 && safety-- > 0) + { + FeedFastWindow(_controller); + } + _controller.CurrentParallelism.Should().Be(2); + var inflatedChunk = _controller.CurrentChunkSizeBytes; + + // Act - Feed a window of slow client-timeout failures (with the new failure kind) + var p = _controller.CurrentParallelism; + for (var i = 0; i < p; i++) + { + RecordUploadResult( + _controller, + TimeSpan.FromSeconds(60), + isSuccess: false, + partNumber: i + 1, + statusCode: 0, + failureKind: UploadFailureKind.ClientTimeout); + } + + RecordUploadResult( + _controller, + TimeSpan.FromSeconds(1), + isSuccess: true, + partNumber: 100); + + // Assert - the timeout samples were ignored and cannot trigger a later downscale + _controller.CurrentChunkSizeBytes.Should().NotBe(500 * 1024); + _controller.CurrentChunkSizeBytes.Should().Be(inflatedChunk, + because: "client-side cancellations are not bandwidth signals and must not influence chunk sizing"); + } + + [Test] + public void ServerError500_StillResetsChunkSize_WhenNoFailureKind() + { + // Arrange - Inflate chunk somewhat first + FeedFastWindow(_controller); + _controller.CurrentChunkSizeBytes.Should().BeGreaterThan(500 * 1024); + + // Act - Record a real 500 server error (unknown failure kind) + RecordUploadResult(_controller, TimeSpan.FromSeconds(2), isSuccess: false, partNumber: 1, statusCode: 500); + + // Assert - resets, like before + _controller.CurrentChunkSizeBytes.Should().Be(500 * 1024); + } + private static void FeedFastWindow(AdaptiveUploadController controller) { FeedWindow(controller, TimeSpan.FromSeconds(10), successes: true); @@ -146,7 +244,23 @@ private static void FeedWindow(AdaptiveUploadController controller, TimeSpan ela var p = controller.CurrentParallelism; for (var i = 0; i < p; i++) { - controller.RecordUploadResult(elapsed, isSuccess: successes, partNumber: i + 1); + RecordUploadResult(controller, elapsed, isSuccess: successes, partNumber: i + 1); } } + + private static void RecordUploadResult( + AdaptiveUploadController controller, + TimeSpan elapsed, + bool isSuccess, + int partNumber, + int? statusCode = null, + UploadFailureKind failureKind = UploadFailureKind.None) + { + controller.RecordUploadResult(new UploadResult( + elapsed, + isSuccess, + partNumber, + statusCode, + FailureKind: failureKind)); + } } \ No newline at end of file diff --git a/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Uploading/FileUploadWorkerTests.cs b/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Uploading/FileUploadWorkerTests.cs index f5665b783..4f2fa3e41 100644 --- a/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Uploading/FileUploadWorkerTests.cs +++ b/tests/ByteSync.Client.UnitTests/Services/Communications/Transfers/Uploading/FileUploadWorkerTests.cs @@ -240,4 +240,105 @@ public async Task UploadAvailableSlicesAdaptiveAsync_ShouldUseCorrectStrategyFor _mockStrategies.Verify(x => x[storageProvider], Times.Once); mockUploadStrategy.Verify(x => x.UploadAsync(slice, mockUploadLocation, It.IsAny()), Times.Once); } + + [Test] + public async Task UploadAvailableSlicesAdaptiveAsync_OnSuccess_ShouldRecordResultWithFailureKindNone() + { + // Arrange + var slice = new FileUploaderSlice(1, new MemoryStream()); + var mockUploadStrategy = new Mock(); + var mockUploadLocation = new FileStorageLocation("https://test.example.com/upload", StorageProvider.CloudflareR2); + + mockUploadStrategy.Setup(x => + x.UploadAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(UploadFileResponse.Success(200)); + + _mockStrategies.Setup(x => x[StorageProvider.CloudflareR2]).Returns(mockUploadStrategy.Object); + _mockFileTransferApiClient.Setup(x => x.GetUploadFileStorageLocation(It.IsAny())) + .ReturnsAsync(mockUploadLocation); + _mockFileTransferApiClient.Setup(x => x.AssertFilePartIsUploaded(It.IsAny())) + .Returns(Task.CompletedTask); + _progressState.TotalCreatedSlices = 1; + + await _availableSlices.Writer.WriteAsync(slice); + _availableSlices.Writer.Complete(); + + // Act + await _fileUploadWorker.UploadAvailableSlicesAdaptiveAsync(_availableSlices, _progressState); + + // Assert + _mockAdaptiveController.Verify(x => x.RecordUploadResult(It.Is(result => + result.IsSuccess && + result.PartNumber == slice.PartNumber && + result.StatusCode == 200 && + result.Exception == null && + result.FileId == _sharedFileDefinition.Id && + result.FailureKind == UploadFailureKind.None)), Times.AtLeastOnce); + } + + [Test] + public async Task UploadAvailableSlicesAdaptiveAsync_OnStrategyClientCancellation_ShouldRecordResultWithClientFailureKind() + { + // Arrange + var slice = new FileUploaderSlice(1, new MemoryStream()); + var mockUploadStrategy = new Mock(); + var mockUploadLocation = new FileStorageLocation("https://test.example.com/upload", StorageProvider.CloudflareR2); + + mockUploadStrategy.Setup(x => + x.UploadAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(UploadFileResponse.ClientCancellation(new TaskCanceledException("attempt timed out"))); + + _mockStrategies.Setup(x => x[StorageProvider.CloudflareR2]).Returns(mockUploadStrategy.Object); + _mockFileTransferApiClient.Setup(x => x.GetUploadFileStorageLocation(It.IsAny())) + .ReturnsAsync(mockUploadLocation); + + await _availableSlices.Writer.WriteAsync(slice); + _availableSlices.Writer.Complete(); + + // Act + await _fileUploadWorker.UploadAvailableSlicesAdaptiveAsync(_availableSlices, _progressState); + + // Assert: a client-side failure kind (Cancellation or Timeout) was reported, never ServerError + _mockAdaptiveController.Verify(x => x.RecordUploadResult(It.Is(result => + !result.IsSuccess && + result.PartNumber == slice.PartNumber && + (result.FailureKind == UploadFailureKind.ClientCancellation || + result.FailureKind == UploadFailureKind.ClientTimeout))), + Times.AtLeastOnce); + + _mockAdaptiveController.Verify(x => x.RecordUploadResult(It.Is(result => + result.FailureKind == UploadFailureKind.ServerError)), + Times.Never); + } + + [Test] + public async Task UploadAvailableSlicesAdaptiveAsync_OnStrategyServerFailure_ShouldRecordResultWithServerErrorKind() + { + // Arrange + var slice = new FileUploaderSlice(1, new MemoryStream()); + var mockUploadStrategy = new Mock(); + var mockUploadLocation = new FileStorageLocation("https://test.example.com/upload", StorageProvider.CloudflareR2); + + mockUploadStrategy.Setup(x => + x.UploadAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(UploadFileResponse.Failure(503, "service unavailable")); + + _mockStrategies.Setup(x => x[StorageProvider.CloudflareR2]).Returns(mockUploadStrategy.Object); + _mockFileTransferApiClient.Setup(x => x.GetUploadFileStorageLocation(It.IsAny())) + .ReturnsAsync(mockUploadLocation); + + await _availableSlices.Writer.WriteAsync(slice); + _availableSlices.Writer.Complete(); + + // Act + await _fileUploadWorker.UploadAvailableSlicesAdaptiveAsync(_availableSlices, _progressState); + + // Assert + _mockAdaptiveController.Verify(x => x.RecordUploadResult(It.Is(result => + !result.IsSuccess && + result.PartNumber == slice.PartNumber && + result.StatusCode == 503 && + result.FileId == _sharedFileDefinition.Id && + result.FailureKind == UploadFailureKind.ServerError)), Times.AtLeastOnce); + } } \ No newline at end of file diff --git a/tests/ByteSync.Client.UnitTests/Uploading/AdaptiveUploadControllerTests.cs b/tests/ByteSync.Client.UnitTests/Uploading/AdaptiveUploadControllerTests.cs index 7f841c035..e59dbc064 100644 --- a/tests/ByteSync.Client.UnitTests/Uploading/AdaptiveUploadControllerTests.cs +++ b/tests/ByteSync.Client.UnitTests/Uploading/AdaptiveUploadControllerTests.cs @@ -1,5 +1,6 @@ using System.Reactive.Linq; using ByteSync.Business.Sessions; +using ByteSync.Common.Business.Communications.Transfers; using ByteSync.Common.Business.Sessions; using ByteSync.Interfaces.Services.Sessions; using ByteSync.Services.Communications.Transfers.Uploading; @@ -39,8 +40,8 @@ public void Upscale_should_increase_chunk_when_window_fast_and_successful() var c = CreateController(); var before = c.CurrentChunkSizeBytes; - c.RecordUploadResult(TimeSpan.FromSeconds(5), true, partNumber: 1); - c.RecordUploadResult(TimeSpan.FromSeconds(6), true, partNumber: 2); + RecordUploadResult(c, TimeSpan.FromSeconds(5), true, partNumber: 1); + RecordUploadResult(c, TimeSpan.FromSeconds(6), true, partNumber: 2); c.CurrentChunkSizeBytes.Should().BeGreaterThan(before); c.CurrentParallelism.Should().Be(2); @@ -51,8 +52,8 @@ public void Downscale_should_reduce_chunk_when_slow_and_at_min_parallelism() { var c = CreateController(); - c.RecordUploadResult(TimeSpan.FromSeconds(35), true, partNumber: 1); - c.RecordUploadResult(TimeSpan.FromSeconds(36), true, partNumber: 2); + RecordUploadResult(c, TimeSpan.FromSeconds(35), true, partNumber: 1); + RecordUploadResult(c, TimeSpan.FromSeconds(36), true, partNumber: 2); // 500 KB * 0.75 = 375 KB c.CurrentChunkSizeBytes.Should().Be(375 * 1024); @@ -64,11 +65,11 @@ public void Error_code_should_reset_chunk_to_initial() { var c = CreateController(); - c.RecordUploadResult(TimeSpan.FromSeconds(1), true, partNumber: 1); - c.RecordUploadResult(TimeSpan.FromSeconds(1), true, partNumber: 2); + RecordUploadResult(c, TimeSpan.FromSeconds(1), true, partNumber: 1); + RecordUploadResult(c, TimeSpan.FromSeconds(1), true, partNumber: 2); c.CurrentChunkSizeBytes.Should().BeGreaterThan(500 * 1024); - c.RecordUploadResult(TimeSpan.FromSeconds(1), false, partNumber: 3, statusCode: 429); + RecordUploadResult(c, TimeSpan.FromSeconds(1), false, partNumber: 3, statusCode: 429); c.CurrentChunkSizeBytes.Should().Be(500 * 1024); } @@ -81,10 +82,20 @@ public void Chunk_size_should_never_exceed_upper_bound() var p = c.CurrentParallelism; for (int j = 0; j < p; j++) { - c.RecordUploadResult(TimeSpan.FromSeconds(1), true, partNumber: i * 10 + j); + RecordUploadResult(c, TimeSpan.FromSeconds(1), true, partNumber: i * 10 + j); } } c.CurrentChunkSizeBytes.Should().BeLessThanOrEqualTo(16 * 1024 * 1024); } + + private static void RecordUploadResult( + AdaptiveUploadController controller, + TimeSpan elapsed, + bool isSuccess, + int partNumber, + int? statusCode = null) + { + controller.RecordUploadResult(new UploadResult(elapsed, isSuccess, partNumber, statusCode)); + } } \ No newline at end of file