From 450ba58e6958e2e697459b4f23519e4204c353fb Mon Sep 17 00:00:00 2001 From: Garrett Beatty Date: Mon, 13 Oct 2025 11:54:21 -0400 Subject: [PATCH 1/6] multipart upload event lifecycles --- .../Internal/MultipartUploadCommand.cs | 38 +++++- .../_async/MultipartUploadCommand.async.cs | 39 ++++-- .../IntegrationTests/TransferUtilityTests.cs | 111 ++++++++++++++++++ 3 files changed, 176 insertions(+), 12 deletions(-) diff --git a/sdk/src/Services/S3/Custom/Transfer/Internal/MultipartUploadCommand.cs b/sdk/src/Services/S3/Custom/Transfer/Internal/MultipartUploadCommand.cs index e31184e6353f..7c81faff1922 100644 --- a/sdk/src/Services/S3/Custom/Transfer/Internal/MultipartUploadCommand.cs +++ b/sdk/src/Services/S3/Custom/Transfer/Internal/MultipartUploadCommand.cs @@ -372,10 +372,46 @@ private void UploadPartProgressEventCallback(object sender, UploadProgressArgs e long transferredBytes = Interlocked.Add(ref _totalTransferredBytes, e.IncrementTransferred - e.CompensationForRetry); var progressArgs = new UploadProgressArgs(e.IncrementTransferred, transferredBytes, this._contentLength, - e.CompensationForRetry, this._fileTransporterRequest.FilePath); + e.CompensationForRetry, this._fileTransporterRequest.FilePath, this._fileTransporterRequest); this._fileTransporterRequest.OnRaiseProgressEvent(progressArgs); } + private void FireTransferInitiatedEvent() + { + var initiatedArgs = new UploadInitiatedEventArgs( + request: _fileTransporterRequest, + filePath: _fileTransporterRequest.FilePath, + totalBytes: _contentLength + ); + + _fileTransporterRequest.OnRaiseTransferInitiatedEvent(initiatedArgs); + } + + private void FireTransferCompletedEvent(TransferUtilityUploadResponse response) + { + var completedArgs = new UploadCompletedEventArgs( + request: _fileTransporterRequest, + response: response, + filePath: _fileTransporterRequest.FilePath, + transferredBytes: Interlocked.Read(ref _totalTransferredBytes), + totalBytes: _contentLength + ); + + _fileTransporterRequest.OnRaiseTransferCompletedEvent(completedArgs); + } + + private void FireTransferFailedEvent() + { + var failedArgs = new UploadFailedEventArgs( + request: _fileTransporterRequest, + filePath: _fileTransporterRequest.FilePath, + transferredBytes: Interlocked.Read(ref _totalTransferredBytes), + totalBytes: _contentLength + ); + + _fileTransporterRequest.OnRaiseTransferFailedEvent(failedArgs); + } + /// /// /// If a checksum algorithm was not specified, we MUST add the default value used by the SDK (as the individual part diff --git a/sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartUploadCommand.async.cs b/sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartUploadCommand.async.cs index 34bd339dc9a3..fc331a8c1fb4 100644 --- a/sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartUploadCommand.async.cs +++ b/sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartUploadCommand.async.cs @@ -33,23 +33,27 @@ internal partial class MultipartUploadCommand : BaseCommand public override async Task ExecuteAsync(CancellationToken cancellationToken) { + // Fire transfer initiated event FIRST, before choosing path + FireTransferInitiatedEvent(); + if ( (this._fileTransporterRequest.InputStream != null && !this._fileTransporterRequest.InputStream.CanSeek) || this._fileTransporterRequest.ContentLength == -1) { await UploadUnseekableStreamAsync(this._fileTransporterRequest, cancellationToken).ConfigureAwait(false); } else { - var initRequest = ConstructInitiateMultipartUploadRequest(); - var initResponse = await _s3Client.InitiateMultipartUploadAsync(initRequest, cancellationToken) - .ConfigureAwait(continueOnCapturedContext: false); - Logger.DebugFormat("Initiated upload: {0}", initResponse.UploadId); - + InitiateMultipartUploadResponse initResponse = null; var pendingUploadPartTasks = new List>(); - SemaphoreSlim localThrottler = null; CancellationTokenSource internalCts = null; + try { + var initRequest = ConstructInitiateMultipartUploadRequest(); + initResponse = await _s3Client.InitiateMultipartUploadAsync(initRequest, cancellationToken) + .ConfigureAwait(continueOnCapturedContext: false); + Logger.DebugFormat("Initiated upload: {0}", initResponse.UploadId); + Logger.DebugFormat("Queue up the UploadPartRequests to be executed"); long filePosition = 0; for (int i = 1; filePosition < this._contentLength; i++) @@ -101,16 +105,24 @@ await localThrottler.WaitAsync(cancellationToken) Logger.DebugFormat("Beginning completing multipart. ({0})", initResponse.UploadId); var compRequest = ConstructCompleteMultipartUploadRequest(initResponse); - await this._s3Client.CompleteMultipartUploadAsync(compRequest, cancellationToken) + var completeResponse = await this._s3Client.CompleteMultipartUploadAsync(compRequest, cancellationToken) .ConfigureAwait(continueOnCapturedContext: false); Logger.DebugFormat("Done completing multipart. ({0})", initResponse.UploadId); + var mappedResponse = ResponseMapper.MapCompleteMultipartUploadResponse(completeResponse); + FireTransferCompletedEvent(mappedResponse); } catch (Exception e) { - Logger.Error(e, "Exception while uploading. ({0})", initResponse.UploadId); - // Can't do async invocation in the catch block, doing cleanup synchronously. - Cleanup(initResponse.UploadId, pendingUploadPartTasks); + Logger.Error(e, "Exception while uploading. ({0})", initResponse?.UploadId ?? "unknown"); + + FireTransferFailedEvent(); + + // Only cleanup if we successfully initiated + if (initResponse != null) + { + Cleanup(initResponse.UploadId, pendingUploadPartTasks); + } throw; } finally @@ -270,12 +282,17 @@ private void AbortMultipartUpload(string uploadId) this._uploadResponses = uploadPartResponses; CompleteMultipartUploadRequest compRequest = ConstructCompleteMultipartUploadRequest(initiateResponse, true, requestEventHandler); - await _s3Client.CompleteMultipartUploadAsync(compRequest, cancellationToken).ConfigureAwait(false); + var completeResponse = await _s3Client.CompleteMultipartUploadAsync(compRequest, cancellationToken).ConfigureAwait(false); Logger.DebugFormat("Completed multi part upload. (Part count: {0}, Upload Id: {1})", uploadPartResponses.Count, initiateResponse.UploadId); + + var mappedResponse = ResponseMapper.MapCompleteMultipartUploadResponse(completeResponse); + FireTransferCompletedEvent(mappedResponse); } } catch (Exception ex) { + FireTransferFailedEvent(); + await _s3Client.AbortMultipartUploadAsync(new AbortMultipartUploadRequest() { BucketName = request.BucketName, diff --git a/sdk/test/Services/S3/IntegrationTests/TransferUtilityTests.cs b/sdk/test/Services/S3/IntegrationTests/TransferUtilityTests.cs index 3280bc13b241..bebf32b59084 100644 --- a/sdk/test/Services/S3/IntegrationTests/TransferUtilityTests.cs +++ b/sdk/test/Services/S3/IntegrationTests/TransferUtilityTests.cs @@ -725,6 +725,117 @@ public void MultipartUploadProgressTest() } } + [TestMethod] + [TestCategory("S3")] + public void MultipartUploadInitiatedEventTest() + { + var fileName = UtilityMethods.GenerateName(@"MultipartUploadTest\InitiatedEvent"); + var eventValidator = new TransferLifecycleEventValidator + { + Validate = (args) => + { + Assert.IsNotNull(args.Request); + Assert.AreEqual(args.FilePath, Path.Combine(BasePath, fileName)); + Assert.IsTrue(args.TotalBytes > 0); + Assert.AreEqual(20 * MEG_SIZE, args.TotalBytes); + } + }; + // Use 20MB+ to trigger multipart upload + UploadWithLifecycleEvents(fileName, 20 * MEG_SIZE, eventValidator, null, null); + eventValidator.AssertEventFired(); + } + + [TestMethod] + [TestCategory("S3")] + public void MultipartUploadCompletedEventTest() + { + var fileName = UtilityMethods.GenerateName(@"MultipartUploadTest\CompletedEvent"); + var eventValidator = new TransferLifecycleEventValidator + { + Validate = (args) => + { + Assert.IsNotNull(args.Request); + Assert.IsNotNull(args.Response); + Assert.AreEqual(args.FilePath, Path.Combine(BasePath, fileName)); + Assert.AreEqual(args.TransferredBytes, args.TotalBytes); + Assert.AreEqual(25 * MEG_SIZE, args.TotalBytes); + Assert.IsTrue(!string.IsNullOrEmpty(args.Response.ETag)); + } + }; + // Use 25MB to trigger multipart upload + UploadWithLifecycleEvents(fileName, 25 * MEG_SIZE, null, eventValidator, null); + eventValidator.AssertEventFired(); + } + + [TestMethod] + [TestCategory("S3")] + public void MultipartUploadFailedEventTest() + { + var fileName = UtilityMethods.GenerateName(@"MultipartUploadTest\FailedEvent"); + var eventValidator = new TransferLifecycleEventValidator + { + Validate = (args) => + { + Assert.IsNotNull(args.Request); + Assert.AreEqual(args.FilePath, Path.Combine(BasePath, fileName)); + Assert.IsTrue(args.TotalBytes > 0); + Assert.AreEqual(22 * MEG_SIZE, args.TotalBytes); + // For failed uploads, transferred bytes should be less than or equal to total bytes + Assert.IsTrue(args.TransferredBytes <= args.TotalBytes); + } + }; + + // Use invalid bucket name to force failure with multipart upload size + var invalidBucketName = "invalid-bucket-name-" + Guid.NewGuid().ToString(); + + try + { + // Use 22MB to trigger multipart upload + UploadWithLifecycleEventsAndBucket(fileName, 22 * MEG_SIZE, invalidBucketName, null, null, eventValidator); + Assert.Fail("Expected an exception to be thrown for invalid bucket"); + } + catch (AmazonS3Exception e) + { + // Expected exception - the failed event should have been fired + eventValidator.AssertEventFired(); + } + } + + [TestMethod] + [TestCategory("S3")] + public void MultipartUploadCompleteLifecycleTest() + { + var fileName = UtilityMethods.GenerateName(@"MultipartUploadTest\CompleteLifecycle"); + + var initiatedValidator = new TransferLifecycleEventValidator + { + Validate = (args) => + { + Assert.IsNotNull(args.Request); + Assert.AreEqual(args.FilePath, Path.Combine(BasePath, fileName)); + Assert.AreEqual(30 * MEG_SIZE, args.TotalBytes); + } + }; + + var completedValidator = new TransferLifecycleEventValidator + { + Validate = (args) => + { + Assert.IsNotNull(args.Request); + Assert.IsNotNull(args.Response); + Assert.AreEqual(args.FilePath, Path.Combine(BasePath, fileName)); + Assert.AreEqual(args.TransferredBytes, args.TotalBytes); + Assert.AreEqual(30 * MEG_SIZE, args.TotalBytes); + } + }; + + // Use 30MB to trigger multipart upload + UploadWithLifecycleEvents(fileName, 30 * MEG_SIZE, initiatedValidator, completedValidator, null); + + initiatedValidator.AssertEventFired(); + completedValidator.AssertEventFired(); + } + [TestMethod] [TestCategory("S3")] public void MultipartGetNumberTest() From e3d7d5c6dd4b6db945d415e322cc1ae6dbf9d3ba Mon Sep 17 00:00:00 2001 From: Garrett Beatty Date: Mon, 13 Oct 2025 14:18:40 -0400 Subject: [PATCH 2/6] add dev config --- .../433a9a6d-b8ea-4676-b763-70711e8288e3.json | 12 ++ .../IntegrationTests/TransferUtilityTests.cs | 160 +++++++++++++++++- 2 files changed, 171 insertions(+), 1 deletion(-) create mode 100644 generator/.DevConfigs/433a9a6d-b8ea-4676-b763-70711e8288e3.json diff --git a/generator/.DevConfigs/433a9a6d-b8ea-4676-b763-70711e8288e3.json b/generator/.DevConfigs/433a9a6d-b8ea-4676-b763-70711e8288e3.json new file mode 100644 index 000000000000..8f2e6f3e7499 --- /dev/null +++ b/generator/.DevConfigs/433a9a6d-b8ea-4676-b763-70711e8288e3.json @@ -0,0 +1,12 @@ +{ + "services": [ + { + "serviceName": "S3", + "type": "patch", + "changeLogMessages": [ + "Added progress tracking events to multipart upload", + "Added CompleteMultipartUploadResponse to TransferUtilityUploadResponse mapping" + ] + } + ] +} diff --git a/sdk/test/Services/S3/IntegrationTests/TransferUtilityTests.cs b/sdk/test/Services/S3/IntegrationTests/TransferUtilityTests.cs index bebf32b59084..801874495ddc 100644 --- a/sdk/test/Services/S3/IntegrationTests/TransferUtilityTests.cs +++ b/sdk/test/Services/S3/IntegrationTests/TransferUtilityTests.cs @@ -794,7 +794,7 @@ public void MultipartUploadFailedEventTest() UploadWithLifecycleEventsAndBucket(fileName, 22 * MEG_SIZE, invalidBucketName, null, null, eventValidator); Assert.Fail("Expected an exception to be thrown for invalid bucket"); } - catch (AmazonS3Exception e) + catch (AmazonS3Exception) { // Expected exception - the failed event should have been fired eventValidator.AssertEventFired(); @@ -836,6 +836,115 @@ public void MultipartUploadCompleteLifecycleTest() completedValidator.AssertEventFired(); } + [TestMethod] + [TestCategory("S3")] + public void MultipartUploadUnseekableStreamInitiatedEventTest() + { + var fileName = UtilityMethods.GenerateName(@"MultipartUploadTest\UnseekableStreamInitiatedEvent"); + var eventValidator = new TransferLifecycleEventValidator + { + Validate = (args) => + { + Assert.IsNotNull(args.Request); + Assert.IsNull(args.FilePath); // No file path for stream uploads + Assert.AreEqual(-1, args.TotalBytes); // Unseekable streams have unknown length + } + }; + // Use 10MB to trigger multipart upload with unseekable stream + UploadUnseekableStreamWithLifecycleEvents(10 * MEG_SIZE, eventValidator, null, null); + eventValidator.AssertEventFired(); + } + + [TestMethod] + [TestCategory("S3")] + public void MultipartUploadUnseekableStreamCompletedEventTest() + { + var fileName = UtilityMethods.GenerateName(@"MultipartUploadTest\UnseekableStreamCompletedEvent"); + var eventValidator = new TransferLifecycleEventValidator + { + Validate = (args) => + { + Assert.IsNotNull(args.Request); + Assert.IsNotNull(args.Response); + Assert.IsNull(args.FilePath); // No file path for stream uploads + Assert.AreEqual(-1, args.TotalBytes); // Unseekable streams have unknown length + Assert.AreEqual(15 * MEG_SIZE, args.TransferredBytes); // since we know the actual length via testing it, we can check the transferredbytes size + Assert.IsTrue(!string.IsNullOrEmpty(args.Response.ETag)); + } + }; + // Use 15MB to trigger multipart upload with unseekable stream + UploadUnseekableStreamWithLifecycleEvents(15 * MEG_SIZE, null, eventValidator, null); + eventValidator.AssertEventFired(); + } + + [TestMethod] + [TestCategory("S3")] + public void MultipartUploadUnseekableStreamFailedEventTest() + { + var fileName = UtilityMethods.GenerateName(@"MultipartUploadTest\UnseekableStreamFailedEvent"); + var eventValidator = new TransferLifecycleEventValidator + { + Validate = (args) => + { + Assert.IsNotNull(args.Request); + Assert.IsNull(args.FilePath); // No file path for stream uploads + Assert.AreEqual(-1, args.TotalBytes); // Unseekable streams have unknown length + // For failed uploads with unseekable streams, transferred bytes should be >= 0 + Assert.IsTrue(args.TransferredBytes >= 0); + } + }; + + // Use invalid bucket name to force failure with multipart upload size + var invalidBucketName = "invalid-bucket-name-" + Guid.NewGuid().ToString(); + + try + { + // Use 12MB to trigger multipart upload with unseekable stream + UploadUnseekableStreamWithLifecycleEventsAndBucket(12 * MEG_SIZE, invalidBucketName, null, null, eventValidator); + Assert.Fail("Expected an exception to be thrown for invalid bucket"); + } + catch (AmazonS3Exception) + { + // Expected exception - the failed event should have been fired + eventValidator.AssertEventFired(); + } + } + + [TestMethod] + [TestCategory("S3")] + public void MultipartUploadUnseekableStreamCompleteLifecycleTest() + { + var fileName = UtilityMethods.GenerateName(@"MultipartUploadTest\UnseekableStreamCompleteLifecycle"); + + var initiatedValidator = new TransferLifecycleEventValidator + { + Validate = (args) => + { + Assert.IsNotNull(args.Request); + Assert.IsNull(args.FilePath); // No file path for stream uploads + Assert.AreEqual(-1, args.TotalBytes); // Unseekable streams have unknown length + } + }; + + var completedValidator = new TransferLifecycleEventValidator + { + Validate = (args) => + { + Assert.IsNotNull(args.Request); + Assert.IsNotNull(args.Response); + Assert.IsNull(args.FilePath); // No file path for stream uploads + Assert.AreEqual(-1, args.TotalBytes); // Unseekable streams have unknown length + Assert.AreEqual(18 * MEG_SIZE, args.TransferredBytes); // Should have transferred all bytes + } + }; + + // Use 18MB to trigger multipart upload with unseekable stream + UploadUnseekableStreamWithLifecycleEvents(18 * MEG_SIZE, initiatedValidator, completedValidator, null); + + initiatedValidator.AssertEventFired(); + completedValidator.AssertEventFired(); + } + [TestMethod] [TestCategory("S3")] public void MultipartGetNumberTest() @@ -1671,6 +1780,55 @@ void UploadWithLifecycleEventsAndBucket(string fileName, long size, string targe transferUtility.Upload(request); } + + void UploadUnseekableStreamWithLifecycleEvents(long size, + TransferLifecycleEventValidator initiatedValidator, + TransferLifecycleEventValidator completedValidator, + TransferLifecycleEventValidator failedValidator) + { + UploadUnseekableStreamWithLifecycleEventsAndBucket(size, bucketName, initiatedValidator, completedValidator, failedValidator); + } + + void UploadUnseekableStreamWithLifecycleEventsAndBucket(long size, string targetBucketName, + TransferLifecycleEventValidator initiatedValidator, + TransferLifecycleEventValidator completedValidator, + TransferLifecycleEventValidator failedValidator) + { + var fileName = UtilityMethods.GenerateName(@"UnseekableStreamUpload\File"); + var key = fileName; + var path = Path.Combine(BasePath, fileName); + UtilityMethods.GenerateFile(path, size); + + // Convert file to unseekable stream + var stream = GenerateUnseekableStreamFromFile(path); + + var config = new TransferUtilityConfig(); + var transferUtility = new TransferUtility(Client, config); + var request = new TransferUtilityUploadRequest + { + BucketName = targetBucketName, + InputStream = stream, + Key = key, + ContentType = octetStreamContentType + }; + + if (initiatedValidator != null) + { + request.UploadInitiatedEvent += initiatedValidator.OnEventFired; + } + + if (completedValidator != null) + { + request.UploadCompletedEvent += completedValidator.OnEventFired; + } + + if (failedValidator != null) + { + request.UploadFailedEvent += failedValidator.OnEventFired; + } + + transferUtility.Upload(request); + } private class UnseekableStream : MemoryStream { private readonly bool _setZeroLengthStream; From 5bf995eb338d184213d3d06568b22fc84cc1c0be Mon Sep 17 00:00:00 2001 From: Garrett Beatty Date: Fri, 17 Oct 2025 13:56:28 -0400 Subject: [PATCH 3/6] fix unseekable stream failure initial request --- .../_async/MultipartUploadCommand.async.cs | 15 +++++++++++++-- .../S3/IntegrationTests/TransferUtilityTests.cs | 12 +++++------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartUploadCommand.async.cs b/sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartUploadCommand.async.cs index fc331a8c1fb4..935218a05d27 100644 --- a/sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartUploadCommand.async.cs +++ b/sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartUploadCommand.async.cs @@ -213,8 +213,19 @@ private void AbortMultipartUpload(string uploadId) } }; - var initiateRequest = ConstructInitiateMultipartUploadRequest(requestEventHandler); - var initiateResponse = await _s3Client.InitiateMultipartUploadAsync(initiateRequest, cancellationToken).ConfigureAwait(false); + InitiateMultipartUploadResponse initiateResponse = null; + + try + { + var initiateRequest = ConstructInitiateMultipartUploadRequest(requestEventHandler); + initiateResponse = await _s3Client.InitiateMultipartUploadAsync(initiateRequest, cancellationToken).ConfigureAwait(false); + } + catch (Exception ex) + { + FireTransferFailedEvent(); + Logger.Error(ex, "Failed to initiate multipart upload for unseekable stream"); + throw; + } try { diff --git a/sdk/test/Services/S3/IntegrationTests/TransferUtilityTests.cs b/sdk/test/Services/S3/IntegrationTests/TransferUtilityTests.cs index 801874495ddc..0a083928432d 100644 --- a/sdk/test/Services/S3/IntegrationTests/TransferUtilityTests.cs +++ b/sdk/test/Services/S3/IntegrationTests/TransferUtilityTests.cs @@ -850,8 +850,7 @@ public void MultipartUploadUnseekableStreamInitiatedEventTest() Assert.AreEqual(-1, args.TotalBytes); // Unseekable streams have unknown length } }; - // Use 10MB to trigger multipart upload with unseekable stream - UploadUnseekableStreamWithLifecycleEvents(10 * MEG_SIZE, eventValidator, null, null); + UploadUnseekableStreamWithLifecycleEvents(20 * MEG_SIZE, eventValidator, null, null); eventValidator.AssertEventFired(); } @@ -868,12 +867,12 @@ public void MultipartUploadUnseekableStreamCompletedEventTest() Assert.IsNotNull(args.Response); Assert.IsNull(args.FilePath); // No file path for stream uploads Assert.AreEqual(-1, args.TotalBytes); // Unseekable streams have unknown length - Assert.AreEqual(15 * MEG_SIZE, args.TransferredBytes); // since we know the actual length via testing it, we can check the transferredbytes size + Assert.AreEqual(20 * MEG_SIZE, args.TransferredBytes); // since we know the actual length via testing it, we can check the transferredbytes size Assert.IsTrue(!string.IsNullOrEmpty(args.Response.ETag)); } }; // Use 15MB to trigger multipart upload with unseekable stream - UploadUnseekableStreamWithLifecycleEvents(15 * MEG_SIZE, null, eventValidator, null); + UploadUnseekableStreamWithLifecycleEvents(20 * MEG_SIZE, null, eventValidator, null); eventValidator.AssertEventFired(); } @@ -889,8 +888,7 @@ public void MultipartUploadUnseekableStreamFailedEventTest() Assert.IsNotNull(args.Request); Assert.IsNull(args.FilePath); // No file path for stream uploads Assert.AreEqual(-1, args.TotalBytes); // Unseekable streams have unknown length - // For failed uploads with unseekable streams, transferred bytes should be >= 0 - Assert.IsTrue(args.TransferredBytes >= 0); + Assert.IsTrue(args.TransferredBytes <= args.TotalBytes); } }; @@ -900,7 +898,7 @@ public void MultipartUploadUnseekableStreamFailedEventTest() try { // Use 12MB to trigger multipart upload with unseekable stream - UploadUnseekableStreamWithLifecycleEventsAndBucket(12 * MEG_SIZE, invalidBucketName, null, null, eventValidator); + UploadUnseekableStreamWithLifecycleEventsAndBucket(20 * MEG_SIZE, invalidBucketName, null, null, eventValidator); Assert.Fail("Expected an exception to be thrown for invalid bucket"); } catch (AmazonS3Exception) From f846aabceea8c156b6cf14acab281cfcc766f75d Mon Sep 17 00:00:00 2001 From: Garrett Beatty Date: Fri, 17 Oct 2025 14:17:37 -0400 Subject: [PATCH 4/6] update try catch --- .../_async/MultipartUploadCommand.async.cs | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartUploadCommand.async.cs b/sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartUploadCommand.async.cs index 935218a05d27..45c6dcdcfc9e 100644 --- a/sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartUploadCommand.async.cs +++ b/sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartUploadCommand.async.cs @@ -43,17 +43,26 @@ public override async Task ExecuteAsync(CancellationToken cancellationToken) else { InitiateMultipartUploadResponse initResponse = null; - var pendingUploadPartTasks = new List>(); - SemaphoreSlim localThrottler = null; - CancellationTokenSource internalCts = null; - try { var initRequest = ConstructInitiateMultipartUploadRequest(); initResponse = await _s3Client.InitiateMultipartUploadAsync(initRequest, cancellationToken) - .ConfigureAwait(continueOnCapturedContext: false); + .ConfigureAwait(continueOnCapturedContext: false); Logger.DebugFormat("Initiated upload: {0}", initResponse.UploadId); + } + catch (Exception e) + { + Logger.Error(e, "Exception while uploading. ({0})", initResponse.UploadId); + FireTransferFailedEvent(); + throw; + } + var pendingUploadPartTasks = new List>(); + SemaphoreSlim localThrottler = null; + CancellationTokenSource internalCts = null; + + try + { Logger.DebugFormat("Queue up the UploadPartRequests to be executed"); long filePosition = 0; for (int i = 1; filePosition < this._contentLength; i++) @@ -118,11 +127,7 @@ await localThrottler.WaitAsync(cancellationToken) FireTransferFailedEvent(); - // Only cleanup if we successfully initiated - if (initResponse != null) - { - Cleanup(initResponse.UploadId, pendingUploadPartTasks); - } + Cleanup(initResponse.UploadId, pendingUploadPartTasks); throw; } finally From 8a4125f0f1282f8130adaee7e4d1225ed6137eaf Mon Sep 17 00:00:00 2001 From: Garrett Beatty Date: Fri, 17 Oct 2025 14:25:52 -0400 Subject: [PATCH 5/6] fix null pointer --- .../Transfer/Internal/_async/MultipartUploadCommand.async.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartUploadCommand.async.cs b/sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartUploadCommand.async.cs index 45c6dcdcfc9e..ceb218c7f8e1 100644 --- a/sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartUploadCommand.async.cs +++ b/sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartUploadCommand.async.cs @@ -50,9 +50,8 @@ public override async Task ExecuteAsync(CancellationToken cancellationToken) .ConfigureAwait(continueOnCapturedContext: false); Logger.DebugFormat("Initiated upload: {0}", initResponse.UploadId); } - catch (Exception e) + catch (Exception) { - Logger.Error(e, "Exception while uploading. ({0})", initResponse.UploadId); FireTransferFailedEvent(); throw; } From be0d3b4583a8ef24696f85b15b731fc084134285 Mon Sep 17 00:00:00 2001 From: Garrett Beatty Date: Fri, 17 Oct 2025 14:28:36 -0400 Subject: [PATCH 6/6] add comment --- .../Transfer/Internal/_async/MultipartUploadCommand.async.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartUploadCommand.async.cs b/sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartUploadCommand.async.cs index ceb218c7f8e1..6e604537daf1 100644 --- a/sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartUploadCommand.async.cs +++ b/sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartUploadCommand.async.cs @@ -123,9 +123,10 @@ await localThrottler.WaitAsync(cancellationToken) catch (Exception e) { Logger.Error(e, "Exception while uploading. ({0})", initResponse?.UploadId ?? "unknown"); - + FireTransferFailedEvent(); + // Can't do async invocation in the catch block, doing cleanup synchronously. Cleanup(initResponse.UploadId, pendingUploadPartTasks); throw; }