Skip to content

Commit 88583d8

Browse files
authored
fix: improve large payload error handling — better error message and prevent infinite retry and fix conflict with auto chunking (#691)
1 parent 095705c commit 88583d8

10 files changed

Lines changed: 624 additions & 42 deletions

File tree

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,3 +351,6 @@ MigrationBackup/
351351

352352
# Rider (cross platform .NET/C# tools) working folder
353353
.idea/
354+
AzuriteConfig
355+
__azurite_db_*
356+
__blobstorage__

samples/LargePayloadConsoleApp/Program.cs

Lines changed: 246 additions & 9 deletions
Large diffs are not rendered by default.
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
namespace Microsoft.DurableTask;
5+
6+
/// <summary>
7+
/// Exception thrown when a payload storage operation fails permanently.
8+
/// </summary>
9+
public sealed class PayloadStorageException : InvalidOperationException
10+
{
11+
/// <summary>
12+
/// Initializes a new instance of the <see cref="PayloadStorageException" /> class.
13+
/// </summary>
14+
/// <param name="message">The error message.</param>
15+
public PayloadStorageException(string message)
16+
: base(message)
17+
{
18+
}
19+
20+
/// <summary>
21+
/// Initializes a new instance of the <see cref="PayloadStorageException" /> class.
22+
/// </summary>
23+
/// <param name="message">The error message.</param>
24+
/// <param name="innerException">The inner exception.</param>
25+
public PayloadStorageException(string message, Exception innerException)
26+
: base(message, innerException)
27+
{
28+
}
29+
}

src/Extensions/AzureBlobPayloads/Interceptors/AzureBlobPayloadsSideCarInterceptor.cs

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4+
using Azure;
45
using Grpc.Core.Interceptors;
56

67
using P = Microsoft.DurableTask.Protobuf;
@@ -39,10 +40,56 @@ protected override async Task ExternalizeRequestPayloadsAsync<TRequest>(TRequest
3940
r.Input = await this.MaybeExternalizeAsync(r.Input, cancellation);
4041
break;
4142
case P.ActivityResponse r:
42-
r.Result = await this.MaybeExternalizeAsync(r.Result, cancellation);
43+
try
44+
{
45+
r.Result = await this.MaybeExternalizeAsync(r.Result, cancellation);
46+
}
47+
catch (Exception ex) when (IsPermanentStorageFailure(ex))
48+
{
49+
// Permanent failure (e.g., payload exceeds configured maximum, 4xx auth/permission error).
50+
// Replace with a failure response so the orchestration sees a failed activity
51+
// instead of the work item being abandoned and redelivered indefinitely.
52+
r.Result = null;
53+
r.FailureDetails = new P.TaskFailureDetails
54+
{
55+
ErrorType = ex.GetType().FullName,
56+
ErrorMessage = ex.Message,
57+
StackTrace = ex.StackTrace,
58+
IsNonRetriable = true,
59+
};
60+
}
61+
4362
break;
4463
case P.OrchestratorResponse r:
45-
await this.ExternalizeOrchestratorResponseAsync(r, cancellation);
64+
try
65+
{
66+
await this.ExternalizeOrchestratorResponseAsync(r, cancellation);
67+
}
68+
catch (Exception ex) when (IsPermanentStorageFailure(ex))
69+
{
70+
// Permanent failure during orchestration response externalization.
71+
// Replace all actions with a single Failed completion so the orchestration
72+
// terminates instead of being abandoned and redelivered indefinitely.
73+
r.Actions.Clear();
74+
r.CustomStatus = null;
75+
r.IsPartial = false;
76+
r.ChunkIndex = null;
77+
r.Actions.Add(new P.OrchestratorAction
78+
{
79+
CompleteOrchestration = new P.CompleteOrchestrationAction
80+
{
81+
OrchestrationStatus = P.OrchestrationStatus.Failed,
82+
FailureDetails = new P.TaskFailureDetails
83+
{
84+
ErrorType = ex.GetType().FullName,
85+
ErrorMessage = ex.Message,
86+
StackTrace = ex.StackTrace,
87+
IsNonRetriable = true,
88+
},
89+
},
90+
});
91+
}
92+
4693
break;
4794
case P.EntityBatchResult r:
4895
await this.ExternalizeEntityBatchResultAsync(r, cancellation);
@@ -360,4 +407,31 @@ async Task ResolveEventPayloadsAsync(P.HistoryEvent e, CancellationToken cancell
360407
break;
361408
}
362409
}
410+
411+
/// <summary>
412+
/// Determines whether an exception represents a permanent storage failure that will never
413+
/// succeed on retry, such as payload exceeding the configured maximum or 4xx HTTP errors
414+
/// (authentication, authorization, not found).
415+
/// </summary>
416+
static bool IsPermanentStorageFailure(Exception ex)
417+
{
418+
if (ex is PayloadStorageException)
419+
{
420+
return true;
421+
}
422+
423+
// Azure SDK retries 408 (Request Timeout) and 429 (Too Many Requests) automatically
424+
// (see ResponseClassifier.IsRetriableResponse in Azure.Core). All other 4xx status codes
425+
// are NOT retried, meaning the request is fundamentally invalid
426+
// (e.g., 401 bad credentials, 403 missing RBAC role, 404 account/container not found).
427+
// These will never succeed on retry with the same configuration.
428+
if (ex is RequestFailedException rfe
429+
&& rfe.Status >= 400 && rfe.Status < 500
430+
&& rfe.Status != 408 && rfe.Status != 429)
431+
{
432+
return true;
433+
}
434+
435+
return false;
436+
}
363437
}

src/Extensions/AzureBlobPayloads/Interceptors/PayloadInterceptor.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -174,17 +174,17 @@ public override AsyncServerStreamingCall<TResponse> AsyncServerStreamingCall<TRe
174174
}
175175

176176
int size = Encoding.UTF8.GetByteCount(value);
177-
if (size < this.options.ExternalizeThresholdBytes)
177+
if (size < this.options.ThresholdBytes)
178178
{
179179
return value;
180180
}
181181

182182
// Enforce a hard cap to prevent unbounded payload sizes
183-
if (size > this.options.MaxExternalizedPayloadBytes)
183+
if (size > this.options.MaxPayloadBytes)
184184
{
185-
throw new InvalidOperationException(
186-
$"Payload size {size / 1024} kb exceeds the configured maximum of {this.options.MaxExternalizedPayloadBytes / 1024} kb. " +
187-
"Consider reducing the payload or increase MaxExternalizedPayloadBytes setting.");
185+
throw new PayloadStorageException(
186+
$"Payload size {size / 1024} KB exceeds the configured maximum of {this.options.MaxPayloadBytes / 1024} KB. " +
187+
"Reduce the payload size or increase the max payload size limit.");
188188
}
189189

190190
return await this.payloadStore.UploadAsync(value!, cancellation);

src/Extensions/AzureBlobPayloads/Options/LargePayloadStorageOptions.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ namespace Microsoft.DurableTask;
2626
/// </summary>
2727
public sealed class LargePayloadStorageOptions
2828
{
29-
int externalizeThresholdBytes = 900_000;
29+
int thresholdBytes = 900_000;
3030

3131
/// <summary>
3232
/// Initializes a new instance of the <see cref="LargePayloadStorageOptions"/> class.
@@ -63,20 +63,20 @@ public LargePayloadStorageOptions(Uri accountUri, TokenCredential credential)
6363
/// Gets or sets the threshold in bytes at which payloads are externalized. Default is 900_000 bytes.
6464
/// Value must not exceed 1 MiB (1,048,576 bytes).
6565
/// </summary>
66-
public int ExternalizeThresholdBytes
66+
public int ThresholdBytes
6767
{
68-
get => this.externalizeThresholdBytes;
68+
get => this.thresholdBytes;
6969
set
7070
{
7171
const int OneMiB = 1 * 1024 * 1024;
7272
if (value > OneMiB)
7373
{
7474
throw new ArgumentOutOfRangeException(
75-
nameof(this.ExternalizeThresholdBytes),
76-
$"ExternalizeThresholdBytes cannot exceed 1 MiB ({OneMiB} bytes).");
75+
nameof(this.ThresholdBytes),
76+
$"Payload storage threshold cannot exceed 1 MiB ({OneMiB} bytes).");
7777
}
7878

79-
this.externalizeThresholdBytes = value;
79+
this.thresholdBytes = value;
8080
}
8181
}
8282

@@ -85,7 +85,7 @@ public int ExternalizeThresholdBytes
8585
/// Defaults to 10MB. Requests exceeding this limit will fail fast
8686
/// with a clear error to prevent unbounded payload growth and excessive storage/network usage.
8787
/// </summary>
88-
public int MaxExternalizedPayloadBytes { get; set; } = 10 * 1024 * 1024;
88+
public int MaxPayloadBytes { get; set; } = 10 * 1024 * 1024;
8989

9090
/// <summary>
9191
/// Gets or sets the Azure Storage connection string to the customer's storage account.
@@ -114,5 +114,5 @@ public int ExternalizeThresholdBytes
114114
/// Gets or sets a value indicating whether payloads should be gzip-compressed when stored.
115115
/// Defaults to true for reduced storage and bandwidth.
116116
/// </summary>
117-
public bool CompressPayloads { get; set; } = true;
117+
public bool CompressionEnabled { get; set; } = true;
118118
}

src/Extensions/AzureBlobPayloads/PayloadStore/BlobPayloadStore.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public BlobPayloadStore(LargePayloadStorageOptions options)
5555
Mode = RetryMode.Exponential,
5656
MaxRetries = MaxRetryAttempts,
5757
Delay = TimeSpan.FromMilliseconds(BaseDelayMs),
58-
MaxDelay = TimeSpan.FromSeconds(MaxDelayMs),
58+
MaxDelay = TimeSpan.FromMilliseconds(MaxDelayMs),
5959
NetworkTimeout = TimeSpan.FromMinutes(NetworkTimeoutMinutes),
6060
},
6161
};
@@ -79,7 +79,7 @@ public override async Task<string> UploadAsync(string payLoad, CancellationToken
7979
// Ensure container exists (idempotent)
8080
await this.containerClient.CreateIfNotExistsAsync(PublicAccessType.None, default, default, cancellationToken);
8181

82-
if (this.options.CompressPayloads)
82+
if (this.options.CompressionEnabled)
8383
{
8484
BlobOpenWriteOptions writeOptions = new()
8585
{
@@ -138,7 +138,7 @@ public override async Task<string> DownloadAsync(string token, CancellationToken
138138
}
139139
catch (RequestFailedException ex) when (ex.Status == (int)HttpStatusCode.NotFound)
140140
{
141-
throw new InvalidOperationException(
141+
throw new PayloadStorageException(
142142
$"The blob '{name}' was not found in container '{container}'. " +
143143
"The payload may have been deleted or the container was never created.",
144144
ex);

src/Worker/Grpc/GrpcDurableTaskWorker.Processor.cs

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -957,7 +957,9 @@ async Task CompleteOrchestratorTaskWithChunkingAsync(
957957
return null;
958958
}
959959

960-
P.TaskFailureDetails? validationFailure = ValidateActionsSize(response.Actions, maxChunkBytes);
960+
P.TaskFailureDetails? validationFailure = this.worker.grpcOptions.Capabilities.Contains(P.WorkerCapability.LargePayloads)
961+
? null
962+
: ValidateActionsSize(response.Actions, maxChunkBytes);
961963
if (validationFailure != null)
962964
{
963965
// Complete the orchestration with a failed status and failure details
@@ -991,7 +993,7 @@ static bool TryAddAction(
991993
int maxChunkBytes)
992994
{
993995
int actionSize = action.CalculateSize();
994-
if (currentSize + actionSize > maxChunkBytes)
996+
if (currentSize + actionSize > maxChunkBytes && currentSize > 0)
995997
{
996998
return false;
997999
}
@@ -1014,6 +1016,7 @@ static bool TryAddAction(
10141016
int actionsCompletedSoFar = 0, chunkIndex = 0;
10151017
List<P.OrchestratorAction> allActions = response.Actions.ToList();
10161018
bool isPartial = true;
1019+
bool isChunkedMode = false;
10171020

10181021
while (isPartial)
10191022
{
@@ -1024,7 +1027,6 @@ static bool TryAddAction(
10241027
CompletionToken = response.CompletionToken,
10251028
RequiresHistory = response.RequiresHistory,
10261029
NumEventsProcessed = 0,
1027-
ChunkIndex = chunkIndex,
10281030
};
10291031

10301032
int chunkPayloadSize = 0;
@@ -1040,6 +1042,20 @@ static bool TryAddAction(
10401042
isPartial = actionsCompletedSoFar < allActions.Count;
10411043
chunkedResponse.IsPartial = isPartial;
10421044

1045+
// Only activate chunked mode when we actually need multiple chunks.
1046+
// A single oversized action that fits in one chunk (via TryAddAction allowing
1047+
// the first item in an empty chunk) should be sent as non-chunked to avoid
1048+
// backend issues with ChunkIndex=0 + IsPartial=false.
1049+
if (isPartial)
1050+
{
1051+
isChunkedMode = true;
1052+
}
1053+
1054+
if (isChunkedMode)
1055+
{
1056+
chunkedResponse.ChunkIndex = chunkIndex;
1057+
}
1058+
10431059
if (chunkIndex == 0)
10441060
{
10451061
// The first chunk preserves the original response's NumEventsProcessed value (null)
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
namespace Microsoft.DurableTask.Grpc.Tests;
5+
6+
public class LargePayloadStorageOptionsTests
7+
{
8+
[Fact]
9+
public void ThresholdBytes_ExactlyOneMiB_DoesNotThrow()
10+
{
11+
// Arrange
12+
LargePayloadStorageOptions options = new();
13+
14+
// Act & Assert
15+
options.ThresholdBytes = 1 * 1024 * 1024;
16+
Assert.Equal(1 * 1024 * 1024, options.ThresholdBytes);
17+
}
18+
19+
[Fact]
20+
public void ThresholdBytes_ExceedsOneMiB_ThrowsArgumentOutOfRange()
21+
{
22+
// Arrange
23+
LargePayloadStorageOptions options = new();
24+
25+
// Act & Assert
26+
ArgumentOutOfRangeException ex = Assert.Throws<ArgumentOutOfRangeException>(
27+
() => options.ThresholdBytes = (1 * 1024 * 1024) + 1);
28+
Assert.Contains("Payload storage threshold", ex.Message);
29+
}
30+
31+
[Fact]
32+
public void ThresholdBytes_DefaultValue_Is900000()
33+
{
34+
// Arrange & Act
35+
LargePayloadStorageOptions options = new();
36+
37+
// Assert
38+
Assert.Equal(900_000, options.ThresholdBytes);
39+
}
40+
41+
[Fact]
42+
public void MaxPayloadBytes_DefaultValue_Is10MB()
43+
{
44+
// Arrange & Act
45+
LargePayloadStorageOptions options = new();
46+
47+
// Assert
48+
Assert.Equal(10 * 1024 * 1024, options.MaxPayloadBytes);
49+
}
50+
51+
[Fact]
52+
public void CompressionEnabled_DefaultValue_IsTrue()
53+
{
54+
// Arrange & Act
55+
LargePayloadStorageOptions options = new();
56+
57+
// Assert
58+
Assert.True(options.CompressionEnabled);
59+
}
60+
61+
[Fact]
62+
public void ContainerName_DefaultValue_IsDurabletaskPayloads()
63+
{
64+
// Arrange & Act
65+
LargePayloadStorageOptions options = new();
66+
67+
// Assert
68+
Assert.Equal("durabletask-payloads", options.ContainerName);
69+
}
70+
}

0 commit comments

Comments
 (0)