feat: Add built-in HTTP activity for standalone SDK#697
feat: Add built-in HTTP activity for standalone SDK#697YunchuWang wants to merge 4 commits intomicrosoft:mainfrom
Conversation
Adds a new extension package (Microsoft.DurableTask.Extensions.Http) that provides a built-in HTTP activity implementation, enabling CallHttpAsync to work in standalone mode without the Azure Functions host. New extension project: src/Extensions/Http/ - DurableHttpRequest/Response — wire-compatible with Azure Functions extension - BuiltInHttpActivity — executes HTTP requests with retry support - UseHttpActivities() — one-line registration on IDurableTaskWorkerBuilder - CallHttpAsync extension methods with 202 async polling - JSON converters for HttpMethod, headers, and TokenSource Closes microsoft#696 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a new extension package that implements the built-in "BuiltIn::HttpActivity" so TaskOrchestrationContext.CallHttpAsync(...) works in standalone worker scenarios (e.g., durabletask-go sidecar) without relying on the Azure Functions host.
Changes:
- Introduce
Microsoft.DurableTask.Extensions.HttpwithDurableHttpRequest/DurableHttpResponse, converters, retry options, and theBuiltInHttpActivityimplementation. - Add worker/orchestrator extension methods:
builder.UseHttpActivities()andTaskOrchestrationContext.CallHttpAsync(...)(including 202 async polling). - Add a new unit test project covering serialization, registration, and activity execution.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Extensions/Http/Http.csproj | New extension project packaging + dependencies + internals visibility for tests |
| src/Extensions/Http/DurableTaskBuilderHttpExtensions.cs | UseHttpActivities() registration for built-in HTTP activity + named HttpClient |
| src/Extensions/Http/TaskOrchestrationContextHttpExtensions.cs | CallHttpAsync orchestration extensions, including 202 polling behavior |
| src/Extensions/Http/BuiltInHttpActivity.cs | Activity implementation that executes HTTP requests and applies HttpRetryOptions |
| src/Extensions/Http/DurableHttpRequest.cs | Wire-compatible durable HTTP request type (+ converters) |
| src/Extensions/Http/DurableHttpResponse.cs | Wire-compatible durable HTTP response type (+ converters) |
| src/Extensions/Http/HttpRetryOptions.cs | Retry policy model for the built-in HTTP activity |
| src/Extensions/Http/TokenSource.cs | TokenSource models for wire-compat (with standalone limitations) |
| src/Extensions/Http/Converters/HttpMethodConverter.cs | JSON conversion for HttpMethod |
| src/Extensions/Http/Converters/HttpHeadersConverter.cs | JSON conversion for header dictionaries (string or string[]) |
| src/Extensions/Http/Converters/TokenSourceConverter.cs | JSON conversion for TokenSource/managed identity model |
| test/Extensions/Http.Tests/Http.Tests.csproj | New test project for the HTTP extension |
| test/Extensions/Http.Tests/BuiltInHttpActivityTests.cs | Unit tests for activity request/response mapping and retries |
| test/Extensions/Http.Tests/SerializationTests.cs | Unit tests for JSON wire round-tripping |
| test/Extensions/Http.Tests/RegistrationTests.cs | Unit tests for DI registration via UseHttpActivities() |
| Directory.Packages.props | Add Microsoft.Extensions.Http package version for shared dependency management |
| Microsoft.DurableTask.sln | Add the new extension + tests to the solution |
| CHANGELOG.md | Document the new HTTP activity extension in Unreleased |
| // Handle 202 async polling pattern | ||
| while (response.StatusCode == HttpStatusCode.Accepted && request.AsynchronousPatternEnabled) | ||
| { |
There was a problem hiding this comment.
DurableHttpRequest.Timeout is documented as the total timeout for the HTTP request and any asynchronous polling, but CallHttpAsync never reads/enforces it. As written, the polling loop can run indefinitely if the endpoint keeps returning 202. Consider tracking a deadline (based on context.CurrentUtcDateTime) and stopping/throwing once the timeout is exceeded.
| var pollRequest = new DurableHttpRequest(HttpMethod.Get, new Uri(locationUrl)) | ||
| { |
There was a problem hiding this comment.
The Location header used for 202 polling may legally be a relative URI (e.g. /status/123). new Uri(locationUrl) will produce a relative Uri, and the subsequent HTTP activity call will fail unless the HttpClient has a BaseAddress. Consider resolving against the original request URI (e.g., new Uri(request.Uri, locationUrl)) before constructing the poll request.
| var pollRequest = new DurableHttpRequest(HttpMethod.Get, new Uri(locationUrl)) | |
| { | |
| Uri pollUri; | |
| if (!Uri.TryCreate(locationUrl, UriKind.Absolute, out pollUri)) | |
| { | |
| if (request.Uri == null || !request.Uri.IsAbsoluteUri) | |
| { | |
| logger.LogWarning( | |
| "HTTP 202 response returned relative 'Location' header '{LocationUrl}', but the original request URI is missing or relative; unable to poll for status.", | |
| locationUrl); | |
| break; | |
| } | |
| pollUri = new Uri(request.Uri, locationUrl); | |
| } | |
| var pollRequest = new DurableHttpRequest(HttpMethod.Get, pollUri) | |
| { |
| if (headers.TryGetValue("Retry-After", out string? retryAfterStr) | ||
| && int.TryParse(retryAfterStr, out int retryAfterSeconds)) | ||
| { | ||
| fireAt = context.CurrentUtcDateTime.AddSeconds(retryAfterSeconds); | ||
| } | ||
| else | ||
| { | ||
| fireAt = context.CurrentUtcDateTime.AddMilliseconds(DefaultPollingIntervalMs); | ||
| } |
There was a problem hiding this comment.
Retry-After can be either delta-seconds or an HTTP-date. The polling logic only supports integer seconds, so valid HTTP-date values will be ignored and fall back to the default interval. Consider also parsing the HTTP-date form to match standard Retry-After semantics.
| if (request.Content != null) | ||
| { | ||
| httpRequest.Content = new StringContent(request.Content, Encoding.UTF8, "application/json"); | ||
| } | ||
|
|
||
| if (request.Headers != null) | ||
| { | ||
| foreach (KeyValuePair<string, string> header in request.Headers) | ||
| { | ||
| // Try request headers first, then content headers | ||
| if (!httpRequest.Headers.TryAddWithoutValidation(header.Key, header.Value)) | ||
| { | ||
| httpRequest.Content?.Headers.TryAddWithoutValidation(header.Key, header.Value); | ||
| } | ||
| } |
There was a problem hiding this comment.
BuildHttpRequest always creates StringContent with media type application/json when request.Content is set. This can override user intent (e.g., form data, plain text, custom Content-Type header) and can also lead to duplicated/contradictory Content-Type headers. Consider not hard-coding the media type here and instead honoring an explicit Content-Type header (or leaving it unset).
| maxAttempts = 1; | ||
| } | ||
|
|
||
| TimeSpan delay = retryOptions?.FirstRetryInterval ?? TimeSpan.Zero; |
There was a problem hiding this comment.
When HttpRetryOptions is provided but FirstRetryInterval is left at its default (TimeSpan.Zero), retries will occur back-to-back with no delay (and exponential backoff will never increase it from zero). Consider validating FirstRetryInterval when MaxNumberOfAttempts > 1, or applying a small, non-zero default delay to avoid tight retry loops.
| TimeSpan delay = retryOptions?.FirstRetryInterval ?? TimeSpan.Zero; | |
| TimeSpan delay = retryOptions?.FirstRetryInterval ?? TimeSpan.Zero; | |
| if (maxAttempts > 1 && delay <= TimeSpan.Zero) | |
| { | |
| delay = TimeSpan.FromSeconds(1); | |
| } |
| public override IDictionary<string, string> Read( | ||
| ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) | ||
| { | ||
| var headers = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase); | ||
|
|
||
| if (reader.TokenType != JsonTokenType.StartObject) | ||
| { | ||
| return headers; | ||
| } | ||
|
|
There was a problem hiding this comment.
HttpHeadersConverter.Read does not handle JsonTokenType.Null. If JSON contains "headers": null, the converter returns an empty dictionary, which changes the meaning for nullable Headers properties (callers cannot distinguish null vs empty) and may break wire-compat expectations. Consider supporting null explicitly (e.g., use a nullable converter JsonConverter<IDictionary<string,string>?> and return null for JsonTokenType.Null).
| /// JSON converter for <see cref="TokenSource"/> — handles serialization only. | ||
| /// Deserialization is not supported since token acquisition is not available in standalone mode. | ||
| /// </summary> | ||
| internal sealed class TokenSourceConverter : JsonConverter<TokenSource> | ||
| { | ||
| /// <inheritdoc/> | ||
| public override TokenSource? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) | ||
| { | ||
| // Skip the token source object during deserialization — token acquisition is not supported. |
There was a problem hiding this comment.
The XML doc comment says this converter “handles serialization only” and that “deserialization is not supported”, but Read(...) is implemented and returns a ManagedIdentityTokenSource. Please update the documentation to reflect the actual behavior (or adjust the implementation to match the documented intent).
| /// JSON converter for <see cref="TokenSource"/> — handles serialization only. | |
| /// Deserialization is not supported since token acquisition is not available in standalone mode. | |
| /// </summary> | |
| internal sealed class TokenSourceConverter : JsonConverter<TokenSource> | |
| { | |
| /// <inheritdoc/> | |
| public override TokenSource? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) | |
| { | |
| // Skip the token source object during deserialization — token acquisition is not supported. | |
| /// JSON converter for <see cref="TokenSource"/>. | |
| /// Supports serialization of <see cref="TokenSource"/> instances and deserialization of managed identity | |
| /// token source payloads into <see cref="ManagedIdentityTokenSource"/>. | |
| /// </summary> | |
| internal sealed class TokenSourceConverter : JsonConverter<TokenSource> | |
| { | |
| /// <inheritdoc/> | |
| public override TokenSource? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) | |
| { | |
| // Handle null values directly. Unrecognized token source payloads are deserialized as null. |
| registry.AddActivity( | ||
| new TaskName(HttpTaskActivityName), | ||
| sp => | ||
| { | ||
| IHttpClientFactory httpClientFactory = sp.GetRequiredService<IHttpClientFactory>(); | ||
| HttpClient client = httpClientFactory.CreateClient("DurableHttp"); | ||
| ILogger logger = sp.GetRequiredService<ILoggerFactory>() | ||
| .CreateLogger("Microsoft.DurableTask.Http.BuiltInHttpActivity"); | ||
| return new BuiltInHttpActivity(client, logger); | ||
| }); |
There was a problem hiding this comment.
UseHttpActivities() unconditionally calls registry.AddActivity(...). Since DurableTaskRegistry.AddActivity throws if the name is already registered, calling UseHttpActivities() in an app that has already registered BuiltIn::HttpActivity (or calling UseHttpActivities() twice) will throw. Consider making registration idempotent or at least surfacing a clearer exception/message about the duplicate registration scenario.
| registry.AddActivity( | |
| new TaskName(HttpTaskActivityName), | |
| sp => | |
| { | |
| IHttpClientFactory httpClientFactory = sp.GetRequiredService<IHttpClientFactory>(); | |
| HttpClient client = httpClientFactory.CreateClient("DurableHttp"); | |
| ILogger logger = sp.GetRequiredService<ILoggerFactory>() | |
| .CreateLogger("Microsoft.DurableTask.Http.BuiltInHttpActivity"); | |
| return new BuiltInHttpActivity(client, logger); | |
| }); | |
| try | |
| { | |
| registry.AddActivity( | |
| new TaskName(HttpTaskActivityName), | |
| sp => | |
| { | |
| IHttpClientFactory httpClientFactory = sp.GetRequiredService<IHttpClientFactory>(); | |
| HttpClient client = httpClientFactory.CreateClient("DurableHttp"); | |
| ILogger logger = sp.GetRequiredService<ILoggerFactory>() | |
| .CreateLogger("Microsoft.DurableTask.Http.BuiltInHttpActivity"); | |
| return new BuiltInHttpActivity(client, logger); | |
| }); | |
| } | |
| catch (System.InvalidOperationException e) | |
| { | |
| throw new System.InvalidOperationException( | |
| $"The built-in HTTP activity '{HttpTaskActivityName}' is already registered. " + | |
| $"This can happen if UseHttpActivities() is called more than once or if the same activity name was registered manually before calling UseHttpActivities().", | |
| e); | |
| } |
| /// <see cref="System.Net.Http.HttpClient"/> to execute HTTP requests. If an <c>IHttpClientFactory</c> | ||
| /// is registered in the service collection, a named client <c>"DurableHttp"</c> is used. | ||
| /// </para> | ||
| /// <para> |
There was a problem hiding this comment.
The remarks state “If an IHttpClientFactory is registered in the service collection, a named client ... is used”, but UseHttpActivities() always registers IHttpClientFactory via AddHttpClient(...) and then requires it via GetRequiredService<IHttpClientFactory>(). Consider updating the remarks to reflect that this extension always uses the named client and will fail if the DI registration is removed/overridden.
| /// <see cref="System.Net.Http.HttpClient"/> to execute HTTP requests. If an <c>IHttpClientFactory</c> | |
| /// is registered in the service collection, a named client <c>"DurableHttp"</c> is used. | |
| /// </para> | |
| /// <para> | |
| /// <see cref="System.Net.Http.HttpClient"/> to execute HTTP requests. This extension also registers | |
| /// and uses a named <see cref="IHttpClientFactory"/> client named <c>"DurableHttp"</c>. | |
| /// </para> | |
| /// <para> | |
| /// The built-in activity resolves <see cref="IHttpClientFactory"/> from the service provider and | |
| /// creates the <c>"DurableHttp"</c> client at runtime. Removing or overriding that DI registration | |
| /// in a way that makes <see cref="IHttpClientFactory"/> unavailable will cause this extension to fail. | |
| /// </para> | |
| /// <para> |
…e URI, retry floor, and test gaps - Enforce DurableHttpRequest.Timeout in 202 polling loop (throw TimeoutException on expiry) - Resolve relative Location URIs against original request URI during 202 polling - Parse Retry-After HTTP-date format (RFC 7231 §7.1.3) in addition to delta-seconds - Honor user-provided Content-Type header instead of hardcoding application/json - Apply 1-second minimum retry interval when FirstRetryInterval is zero to prevent tight loops - Dispose HttpResponseMessage after reading body (fix resource leak) - Handle null JSON in HttpHeadersConverter; join multi-value arrays with ", " - Fix TokenSourceConverter doc comment to match actual deserialization behavior - Make UseHttpActivities() idempotent (safe to call twice) - Update UseHttpActivities() remarks for accuracy about IHttpClientFactory - Add 6 new tests: custom Content-Type, default Content-Type, zero retry interval, null headers deserialization, multi-value header arrays, double UseHttpActivities() All 22 tests passing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ed feature The previous doc comment implied managed identity deserialization was a supported capability. In reality, TokenSource types exist solely for wire compatibility — BuiltInHttpActivity throws NotSupportedException at runtime if a TokenSource is present. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| [JsonPropertyName("FirstRetryInterval")] | ||
| public TimeSpan FirstRetryInterval { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the max retry interval. Defaults to 6 days. | ||
| /// </summary> | ||
| [JsonPropertyName("MaxRetryInterval")] | ||
| public TimeSpan MaxRetryInterval { get; set; } = DefaultMaxRetryInterval; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the backoff coefficient. Defaults to 1. | ||
| /// </summary> | ||
| [JsonPropertyName("BackoffCoefficient")] | ||
| public double BackoffCoefficient { get; set; } = 1; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the timeout for retries. Defaults to <see cref="TimeSpan.MaxValue"/>. | ||
| /// </summary> | ||
| [JsonPropertyName("RetryTimeout")] | ||
| public TimeSpan RetryTimeout { get; set; } = TimeSpan.MaxValue; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the max number of attempts. | ||
| /// </summary> | ||
| [JsonPropertyName("MaxNumberOfAttempts")] | ||
| public int MaxNumberOfAttempts { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets the list of status codes to retry on. If empty, all 4xx and 5xx are retried. | ||
| /// </summary> | ||
| [JsonPropertyName("StatusCodesToRetry")] |
There was a problem hiding this comment.
The JSON property names for HttpRetryOptions use PascalCase (e.g., "FirstRetryInterval"), while the rest of the HTTP wire types in this PR use camelCase (e.g., DurableHttpRequest.retryOptions, DurableHttpResponse.statusCode). This likely breaks the “wire-compatible with the Azure Functions extension” claim because payloads from Functions are typically camelCase. Consider switching these JsonPropertyName values to camelCase (e.g., firstRetryInterval, maxRetryInterval, etc.) to align with the expected wire format.
| [JsonPropertyName("FirstRetryInterval")] | |
| public TimeSpan FirstRetryInterval { get; set; } | |
| /// <summary> | |
| /// Gets or sets the max retry interval. Defaults to 6 days. | |
| /// </summary> | |
| [JsonPropertyName("MaxRetryInterval")] | |
| public TimeSpan MaxRetryInterval { get; set; } = DefaultMaxRetryInterval; | |
| /// <summary> | |
| /// Gets or sets the backoff coefficient. Defaults to 1. | |
| /// </summary> | |
| [JsonPropertyName("BackoffCoefficient")] | |
| public double BackoffCoefficient { get; set; } = 1; | |
| /// <summary> | |
| /// Gets or sets the timeout for retries. Defaults to <see cref="TimeSpan.MaxValue"/>. | |
| /// </summary> | |
| [JsonPropertyName("RetryTimeout")] | |
| public TimeSpan RetryTimeout { get; set; } = TimeSpan.MaxValue; | |
| /// <summary> | |
| /// Gets or sets the max number of attempts. | |
| /// </summary> | |
| [JsonPropertyName("MaxNumberOfAttempts")] | |
| public int MaxNumberOfAttempts { get; set; } | |
| /// <summary> | |
| /// Gets the list of status codes to retry on. If empty, all 4xx and 5xx are retried. | |
| /// </summary> | |
| [JsonPropertyName("StatusCodesToRetry")] | |
| [JsonPropertyName("firstRetryInterval")] | |
| public TimeSpan FirstRetryInterval { get; set; } | |
| /// <summary> | |
| /// Gets or sets the max retry interval. Defaults to 6 days. | |
| /// </summary> | |
| [JsonPropertyName("maxRetryInterval")] | |
| public TimeSpan MaxRetryInterval { get; set; } = DefaultMaxRetryInterval; | |
| /// <summary> | |
| /// Gets or sets the backoff coefficient. Defaults to 1. | |
| /// </summary> | |
| [JsonPropertyName("backoffCoefficient")] | |
| public double BackoffCoefficient { get; set; } = 1; | |
| /// <summary> | |
| /// Gets or sets the timeout for retries. Defaults to <see cref="TimeSpan.MaxValue"/>. | |
| /// </summary> | |
| [JsonPropertyName("retryTimeout")] | |
| public TimeSpan RetryTimeout { get; set; } = TimeSpan.MaxValue; | |
| /// <summary> | |
| /// Gets or sets the max number of attempts. | |
| /// </summary> | |
| [JsonPropertyName("maxNumberOfAttempts")] | |
| public int MaxNumberOfAttempts { get; set; } | |
| /// <summary> | |
| /// Gets the list of status codes to retry on. If empty, all 4xx and 5xx are retried. | |
| /// </summary> | |
| [JsonPropertyName("statusCodesToRetry")] |
| [JsonPropertyName("FirstRetryInterval")] | ||
| public TimeSpan FirstRetryInterval { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the max retry interval. Defaults to 6 days. | ||
| /// </summary> | ||
| [JsonPropertyName("MaxRetryInterval")] | ||
| public TimeSpan MaxRetryInterval { get; set; } = DefaultMaxRetryInterval; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the backoff coefficient. Defaults to 1. | ||
| /// </summary> | ||
| [JsonPropertyName("BackoffCoefficient")] | ||
| public double BackoffCoefficient { get; set; } = 1; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the timeout for retries. Defaults to <see cref="TimeSpan.MaxValue"/>. | ||
| /// </summary> | ||
| [JsonPropertyName("RetryTimeout")] | ||
| public TimeSpan RetryTimeout { get; set; } = TimeSpan.MaxValue; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the max number of attempts. | ||
| /// </summary> | ||
| [JsonPropertyName("MaxNumberOfAttempts")] | ||
| public int MaxNumberOfAttempts { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets the list of status codes to retry on. If empty, all 4xx and 5xx are retried. | ||
| /// </summary> | ||
| [JsonPropertyName("StatusCodesToRetry")] |
There was a problem hiding this comment.
The JSON property names for HttpRetryOptions use PascalCase (e.g., "FirstRetryInterval"), while the rest of the HTTP wire types in this PR use camelCase (e.g., DurableHttpRequest.retryOptions, DurableHttpResponse.statusCode). This likely breaks the “wire-compatible with the Azure Functions extension” claim because payloads from Functions are typically camelCase. Consider switching these JsonPropertyName values to camelCase (e.g., firstRetryInterval, maxRetryInterval, etc.) to align with the expected wire format.
| [JsonPropertyName("FirstRetryInterval")] | |
| public TimeSpan FirstRetryInterval { get; set; } | |
| /// <summary> | |
| /// Gets or sets the max retry interval. Defaults to 6 days. | |
| /// </summary> | |
| [JsonPropertyName("MaxRetryInterval")] | |
| public TimeSpan MaxRetryInterval { get; set; } = DefaultMaxRetryInterval; | |
| /// <summary> | |
| /// Gets or sets the backoff coefficient. Defaults to 1. | |
| /// </summary> | |
| [JsonPropertyName("BackoffCoefficient")] | |
| public double BackoffCoefficient { get; set; } = 1; | |
| /// <summary> | |
| /// Gets or sets the timeout for retries. Defaults to <see cref="TimeSpan.MaxValue"/>. | |
| /// </summary> | |
| [JsonPropertyName("RetryTimeout")] | |
| public TimeSpan RetryTimeout { get; set; } = TimeSpan.MaxValue; | |
| /// <summary> | |
| /// Gets or sets the max number of attempts. | |
| /// </summary> | |
| [JsonPropertyName("MaxNumberOfAttempts")] | |
| public int MaxNumberOfAttempts { get; set; } | |
| /// <summary> | |
| /// Gets the list of status codes to retry on. If empty, all 4xx and 5xx are retried. | |
| /// </summary> | |
| [JsonPropertyName("StatusCodesToRetry")] | |
| [JsonPropertyName("firstRetryInterval")] | |
| public TimeSpan FirstRetryInterval { get; set; } | |
| /// <summary> | |
| /// Gets or sets the max retry interval. Defaults to 6 days. | |
| /// </summary> | |
| [JsonPropertyName("maxRetryInterval")] | |
| public TimeSpan MaxRetryInterval { get; set; } = DefaultMaxRetryInterval; | |
| /// <summary> | |
| /// Gets or sets the backoff coefficient. Defaults to 1. | |
| /// </summary> | |
| [JsonPropertyName("backoffCoefficient")] | |
| public double BackoffCoefficient { get; set; } = 1; | |
| /// <summary> | |
| /// Gets or sets the timeout for retries. Defaults to <see cref="TimeSpan.MaxValue"/>. | |
| /// </summary> | |
| [JsonPropertyName("retryTimeout")] | |
| public TimeSpan RetryTimeout { get; set; } = TimeSpan.MaxValue; | |
| /// <summary> | |
| /// Gets or sets the max number of attempts. | |
| /// </summary> | |
| [JsonPropertyName("maxNumberOfAttempts")] | |
| public int MaxNumberOfAttempts { get; set; } | |
| /// <summary> | |
| /// Gets the list of status codes to retry on. If empty, all 4xx and 5xx are retried. | |
| /// </summary> | |
| [JsonPropertyName("statusCodesToRetry")] |
| [JsonPropertyName("FirstRetryInterval")] | ||
| public TimeSpan FirstRetryInterval { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the max retry interval. Defaults to 6 days. | ||
| /// </summary> | ||
| [JsonPropertyName("MaxRetryInterval")] | ||
| public TimeSpan MaxRetryInterval { get; set; } = DefaultMaxRetryInterval; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the backoff coefficient. Defaults to 1. | ||
| /// </summary> | ||
| [JsonPropertyName("BackoffCoefficient")] | ||
| public double BackoffCoefficient { get; set; } = 1; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the timeout for retries. Defaults to <see cref="TimeSpan.MaxValue"/>. | ||
| /// </summary> | ||
| [JsonPropertyName("RetryTimeout")] | ||
| public TimeSpan RetryTimeout { get; set; } = TimeSpan.MaxValue; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the max number of attempts. | ||
| /// </summary> | ||
| [JsonPropertyName("MaxNumberOfAttempts")] | ||
| public int MaxNumberOfAttempts { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets the list of status codes to retry on. If empty, all 4xx and 5xx are retried. | ||
| /// </summary> | ||
| [JsonPropertyName("StatusCodesToRetry")] |
There was a problem hiding this comment.
The JSON property names for HttpRetryOptions use PascalCase (e.g., "FirstRetryInterval"), while the rest of the HTTP wire types in this PR use camelCase (e.g., DurableHttpRequest.retryOptions, DurableHttpResponse.statusCode). This likely breaks the “wire-compatible with the Azure Functions extension” claim because payloads from Functions are typically camelCase. Consider switching these JsonPropertyName values to camelCase (e.g., firstRetryInterval, maxRetryInterval, etc.) to align with the expected wire format.
| [JsonPropertyName("FirstRetryInterval")] | |
| public TimeSpan FirstRetryInterval { get; set; } | |
| /// <summary> | |
| /// Gets or sets the max retry interval. Defaults to 6 days. | |
| /// </summary> | |
| [JsonPropertyName("MaxRetryInterval")] | |
| public TimeSpan MaxRetryInterval { get; set; } = DefaultMaxRetryInterval; | |
| /// <summary> | |
| /// Gets or sets the backoff coefficient. Defaults to 1. | |
| /// </summary> | |
| [JsonPropertyName("BackoffCoefficient")] | |
| public double BackoffCoefficient { get; set; } = 1; | |
| /// <summary> | |
| /// Gets or sets the timeout for retries. Defaults to <see cref="TimeSpan.MaxValue"/>. | |
| /// </summary> | |
| [JsonPropertyName("RetryTimeout")] | |
| public TimeSpan RetryTimeout { get; set; } = TimeSpan.MaxValue; | |
| /// <summary> | |
| /// Gets or sets the max number of attempts. | |
| /// </summary> | |
| [JsonPropertyName("MaxNumberOfAttempts")] | |
| public int MaxNumberOfAttempts { get; set; } | |
| /// <summary> | |
| /// Gets the list of status codes to retry on. If empty, all 4xx and 5xx are retried. | |
| /// </summary> | |
| [JsonPropertyName("StatusCodesToRetry")] | |
| [JsonPropertyName("firstRetryInterval")] | |
| public TimeSpan FirstRetryInterval { get; set; } | |
| /// <summary> | |
| /// Gets or sets the max retry interval. Defaults to 6 days. | |
| /// </summary> | |
| [JsonPropertyName("maxRetryInterval")] | |
| public TimeSpan MaxRetryInterval { get; set; } = DefaultMaxRetryInterval; | |
| /// <summary> | |
| /// Gets or sets the backoff coefficient. Defaults to 1. | |
| /// </summary> | |
| [JsonPropertyName("backoffCoefficient")] | |
| public double BackoffCoefficient { get; set; } = 1; | |
| /// <summary> | |
| /// Gets or sets the timeout for retries. Defaults to <see cref="TimeSpan.MaxValue"/>. | |
| /// </summary> | |
| [JsonPropertyName("retryTimeout")] | |
| public TimeSpan RetryTimeout { get; set; } = TimeSpan.MaxValue; | |
| /// <summary> | |
| /// Gets or sets the max number of attempts. | |
| /// </summary> | |
| [JsonPropertyName("maxNumberOfAttempts")] | |
| public int MaxNumberOfAttempts { get; set; } | |
| /// <summary> | |
| /// Gets the list of status codes to retry on. If empty, all 4xx and 5xx are retried. | |
| /// </summary> | |
| [JsonPropertyName("statusCodesToRetry")] |
| [JsonPropertyName("FirstRetryInterval")] | ||
| public TimeSpan FirstRetryInterval { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the max retry interval. Defaults to 6 days. | ||
| /// </summary> | ||
| [JsonPropertyName("MaxRetryInterval")] | ||
| public TimeSpan MaxRetryInterval { get; set; } = DefaultMaxRetryInterval; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the backoff coefficient. Defaults to 1. | ||
| /// </summary> | ||
| [JsonPropertyName("BackoffCoefficient")] | ||
| public double BackoffCoefficient { get; set; } = 1; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the timeout for retries. Defaults to <see cref="TimeSpan.MaxValue"/>. | ||
| /// </summary> | ||
| [JsonPropertyName("RetryTimeout")] | ||
| public TimeSpan RetryTimeout { get; set; } = TimeSpan.MaxValue; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the max number of attempts. | ||
| /// </summary> | ||
| [JsonPropertyName("MaxNumberOfAttempts")] | ||
| public int MaxNumberOfAttempts { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets the list of status codes to retry on. If empty, all 4xx and 5xx are retried. | ||
| /// </summary> | ||
| [JsonPropertyName("StatusCodesToRetry")] |
There was a problem hiding this comment.
The JSON property names for HttpRetryOptions use PascalCase (e.g., "FirstRetryInterval"), while the rest of the HTTP wire types in this PR use camelCase (e.g., DurableHttpRequest.retryOptions, DurableHttpResponse.statusCode). This likely breaks the “wire-compatible with the Azure Functions extension” claim because payloads from Functions are typically camelCase. Consider switching these JsonPropertyName values to camelCase (e.g., firstRetryInterval, maxRetryInterval, etc.) to align with the expected wire format.
| [JsonPropertyName("FirstRetryInterval")] | |
| public TimeSpan FirstRetryInterval { get; set; } | |
| /// <summary> | |
| /// Gets or sets the max retry interval. Defaults to 6 days. | |
| /// </summary> | |
| [JsonPropertyName("MaxRetryInterval")] | |
| public TimeSpan MaxRetryInterval { get; set; } = DefaultMaxRetryInterval; | |
| /// <summary> | |
| /// Gets or sets the backoff coefficient. Defaults to 1. | |
| /// </summary> | |
| [JsonPropertyName("BackoffCoefficient")] | |
| public double BackoffCoefficient { get; set; } = 1; | |
| /// <summary> | |
| /// Gets or sets the timeout for retries. Defaults to <see cref="TimeSpan.MaxValue"/>. | |
| /// </summary> | |
| [JsonPropertyName("RetryTimeout")] | |
| public TimeSpan RetryTimeout { get; set; } = TimeSpan.MaxValue; | |
| /// <summary> | |
| /// Gets or sets the max number of attempts. | |
| /// </summary> | |
| [JsonPropertyName("MaxNumberOfAttempts")] | |
| public int MaxNumberOfAttempts { get; set; } | |
| /// <summary> | |
| /// Gets the list of status codes to retry on. If empty, all 4xx and 5xx are retried. | |
| /// </summary> | |
| [JsonPropertyName("StatusCodesToRetry")] | |
| [JsonPropertyName("firstRetryInterval")] | |
| public TimeSpan FirstRetryInterval { get; set; } | |
| /// <summary> | |
| /// Gets or sets the max retry interval. Defaults to 6 days. | |
| /// </summary> | |
| [JsonPropertyName("maxRetryInterval")] | |
| public TimeSpan MaxRetryInterval { get; set; } = DefaultMaxRetryInterval; | |
| /// <summary> | |
| /// Gets or sets the backoff coefficient. Defaults to 1. | |
| /// </summary> | |
| [JsonPropertyName("backoffCoefficient")] | |
| public double BackoffCoefficient { get; set; } = 1; | |
| /// <summary> | |
| /// Gets or sets the timeout for retries. Defaults to <see cref="TimeSpan.MaxValue"/>. | |
| /// </summary> | |
| [JsonPropertyName("retryTimeout")] | |
| public TimeSpan RetryTimeout { get; set; } = TimeSpan.MaxValue; | |
| /// <summary> | |
| /// Gets or sets the max number of attempts. | |
| /// </summary> | |
| [JsonPropertyName("maxNumberOfAttempts")] | |
| public int MaxNumberOfAttempts { get; set; } | |
| /// <summary> | |
| /// Gets the list of status codes to retry on. If empty, all 4xx and 5xx are retried. | |
| /// </summary> | |
| [JsonPropertyName("statusCodesToRetry")] |
| [JsonPropertyName("FirstRetryInterval")] | ||
| public TimeSpan FirstRetryInterval { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the max retry interval. Defaults to 6 days. | ||
| /// </summary> | ||
| [JsonPropertyName("MaxRetryInterval")] | ||
| public TimeSpan MaxRetryInterval { get; set; } = DefaultMaxRetryInterval; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the backoff coefficient. Defaults to 1. | ||
| /// </summary> | ||
| [JsonPropertyName("BackoffCoefficient")] | ||
| public double BackoffCoefficient { get; set; } = 1; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the timeout for retries. Defaults to <see cref="TimeSpan.MaxValue"/>. | ||
| /// </summary> | ||
| [JsonPropertyName("RetryTimeout")] | ||
| public TimeSpan RetryTimeout { get; set; } = TimeSpan.MaxValue; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the max number of attempts. | ||
| /// </summary> | ||
| [JsonPropertyName("MaxNumberOfAttempts")] | ||
| public int MaxNumberOfAttempts { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets the list of status codes to retry on. If empty, all 4xx and 5xx are retried. | ||
| /// </summary> | ||
| [JsonPropertyName("StatusCodesToRetry")] |
There was a problem hiding this comment.
The JSON property names for HttpRetryOptions use PascalCase (e.g., "FirstRetryInterval"), while the rest of the HTTP wire types in this PR use camelCase (e.g., DurableHttpRequest.retryOptions, DurableHttpResponse.statusCode). This likely breaks the “wire-compatible with the Azure Functions extension” claim because payloads from Functions are typically camelCase. Consider switching these JsonPropertyName values to camelCase (e.g., firstRetryInterval, maxRetryInterval, etc.) to align with the expected wire format.
| [JsonPropertyName("FirstRetryInterval")] | |
| public TimeSpan FirstRetryInterval { get; set; } | |
| /// <summary> | |
| /// Gets or sets the max retry interval. Defaults to 6 days. | |
| /// </summary> | |
| [JsonPropertyName("MaxRetryInterval")] | |
| public TimeSpan MaxRetryInterval { get; set; } = DefaultMaxRetryInterval; | |
| /// <summary> | |
| /// Gets or sets the backoff coefficient. Defaults to 1. | |
| /// </summary> | |
| [JsonPropertyName("BackoffCoefficient")] | |
| public double BackoffCoefficient { get; set; } = 1; | |
| /// <summary> | |
| /// Gets or sets the timeout for retries. Defaults to <see cref="TimeSpan.MaxValue"/>. | |
| /// </summary> | |
| [JsonPropertyName("RetryTimeout")] | |
| public TimeSpan RetryTimeout { get; set; } = TimeSpan.MaxValue; | |
| /// <summary> | |
| /// Gets or sets the max number of attempts. | |
| /// </summary> | |
| [JsonPropertyName("MaxNumberOfAttempts")] | |
| public int MaxNumberOfAttempts { get; set; } | |
| /// <summary> | |
| /// Gets the list of status codes to retry on. If empty, all 4xx and 5xx are retried. | |
| /// </summary> | |
| [JsonPropertyName("StatusCodesToRetry")] | |
| [JsonPropertyName("firstRetryInterval")] | |
| public TimeSpan FirstRetryInterval { get; set; } | |
| /// <summary> | |
| /// Gets or sets the max retry interval. Defaults to 6 days. | |
| /// </summary> | |
| [JsonPropertyName("maxRetryInterval")] | |
| public TimeSpan MaxRetryInterval { get; set; } = DefaultMaxRetryInterval; | |
| /// <summary> | |
| /// Gets or sets the backoff coefficient. Defaults to 1. | |
| /// </summary> | |
| [JsonPropertyName("backoffCoefficient")] | |
| public double BackoffCoefficient { get; set; } = 1; | |
| /// <summary> | |
| /// Gets or sets the timeout for retries. Defaults to <see cref="TimeSpan.MaxValue"/>. | |
| /// </summary> | |
| [JsonPropertyName("retryTimeout")] | |
| public TimeSpan RetryTimeout { get; set; } = TimeSpan.MaxValue; | |
| /// <summary> | |
| /// Gets or sets the max number of attempts. | |
| /// </summary> | |
| [JsonPropertyName("maxNumberOfAttempts")] | |
| public int MaxNumberOfAttempts { get; set; } | |
| /// <summary> | |
| /// Gets the list of status codes to retry on. If empty, all 4xx and 5xx are retried. | |
| /// </summary> | |
| [JsonPropertyName("statusCodesToRetry")] |
| // Skip Content-Type — already set via StringContent constructor above | ||
| if (string.Equals(header.Key, "Content-Type", StringComparison.OrdinalIgnoreCase)) |
There was a problem hiding this comment.
Content-Type is always skipped when copying headers, even when request.Content is null (i.e., when httpRequest.Content was never created). This can silently drop a user-specified Content-Type header on requests without a body. Consider skipping Content-Type only when httpRequest.Content != null (or otherwise ensuring the header is preserved when there is no content).
| // Skip Content-Type — already set via StringContent constructor above | |
| if (string.Equals(header.Key, "Content-Type", StringComparison.OrdinalIgnoreCase)) | |
| // Skip Content-Type only when it was already set via the StringContent constructor above. | |
| if (httpRequest.Content != null && | |
| string.Equals(header.Key, "Content-Type", StringComparison.OrdinalIgnoreCase)) |
| protected override Task<HttpResponseMessage> SendAsync( | ||
| HttpRequestMessage request, CancellationToken cancellationToken) | ||
| { | ||
| this.RequestsSent.Add(request); | ||
| this.RequestBodies.Add(request.Content?.ReadAsStringAsync().Result); | ||
|
|
||
| if (this.responses.Count == 0) | ||
| { | ||
| return Task.FromResult(new HttpResponseMessage(HttpStatusCode.InternalServerError)); | ||
| } | ||
|
|
||
| return Task.FromResult(this.responses.Dequeue()); |
There was a problem hiding this comment.
The mock handler uses .Result on an async API inside SendAsync, which is a sync-over-async pattern that can deadlock and also ignores the provided cancellationToken. Consider making SendAsync async and awaiting ReadAsStringAsync(cancellationToken) (or at least ReadAsStringAsync() without blocking).
| protected override Task<HttpResponseMessage> SendAsync( | |
| HttpRequestMessage request, CancellationToken cancellationToken) | |
| { | |
| this.RequestsSent.Add(request); | |
| this.RequestBodies.Add(request.Content?.ReadAsStringAsync().Result); | |
| if (this.responses.Count == 0) | |
| { | |
| return Task.FromResult(new HttpResponseMessage(HttpStatusCode.InternalServerError)); | |
| } | |
| return Task.FromResult(this.responses.Dequeue()); | |
| protected override async Task<HttpResponseMessage> SendAsync( | |
| HttpRequestMessage request, CancellationToken cancellationToken) | |
| { | |
| this.RequestsSent.Add(request); | |
| string? requestBody = request.Content is null | |
| ? null | |
| : await request.Content.ReadAsStringAsync(cancellationToken); | |
| this.RequestBodies.Add(requestBody); | |
| if (this.responses.Count == 0) | |
| { | |
| return new HttpResponseMessage(HttpStatusCode.InternalServerError); | |
| } | |
| return this.responses.Dequeue(); |
| var headers = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase); | ||
|
|
||
| if (reader.TokenType != JsonTokenType.StartObject) | ||
| { | ||
| return headers; | ||
| } | ||
|
|
There was a problem hiding this comment.
For non-null, non-object JSON (e.g., a string/array/number), the converter silently returns an empty dictionary. This can mask malformed payloads and make debugging wire issues harder. Consider throwing JsonException when the token type isn’t StartObject (or explicitly documenting/justifying why invalid types should deserialize as “empty headers”).
| var headers = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase); | |
| if (reader.TokenType != JsonTokenType.StartObject) | |
| { | |
| return headers; | |
| } | |
| if (reader.TokenType != JsonTokenType.StartObject) | |
| { | |
| throw new JsonException( | |
| $"Expected {JsonTokenType.StartObject} or {JsonTokenType.Null} when deserializing HTTP headers, but found {reader.TokenType}."); | |
| } | |
| var headers = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase); |
|
|
||
| if (root.TryGetProperty("resource", out JsonElement resourceElement)) | ||
| { | ||
| string resource = resourceElement.GetString() ?? string.Empty; |
There was a problem hiding this comment.
If "resource" is present but null (or an empty string), this code constructs a ManagedIdentityTokenSource with an empty resource, which likely isn’t a valid wire payload and can lead to confusing behavior later. Consider treating null/empty resource as an invalid payload (throw JsonException) or deserializing to null to match the “unrecognized payloads deserialize as null” behavior.
| string resource = resourceElement.GetString() ?? string.Empty; | |
| if (resourceElement.ValueKind != JsonValueKind.String) | |
| { | |
| return null; | |
| } | |
| string? resource = resourceElement.GetString(); | |
| if (string.IsNullOrEmpty(resource)) | |
| { | |
| return null; | |
| } |
| opts = new ManagedIdentityOptions(authorityHost, tenantId); | ||
| } | ||
|
|
||
| return new ManagedIdentityTokenSource(resource, opts); |
There was a problem hiding this comment.
If "resource" is present but null (or an empty string), this code constructs a ManagedIdentityTokenSource with an empty resource, which likely isn’t a valid wire payload and can lead to confusing behavior later. Consider treating null/empty resource as an invalid payload (throw JsonException) or deserializing to null to match the “unrecognized payloads deserialize as null” behavior.
Summary
Microsoft.DurableTask.Extensions.Httpthat implementsBuiltIn::HttpActivity, enablingCallHttpAsyncto work in standalone mode (e.g., with durabletask-go sidecar) without requiring the Azure Functions hostDurableHttpRequest/DurableHttpResponsetypes wire-compatible with the Azure Functions extension, retry support viaHttpRetryOptions, and 202 async pollingbuilder.UseHttpActivities()onIDurableTaskWorkerBuilderMotivation
In standalone scenarios (durabletask-dotnet + durabletask-go sidecar),
CallHttpAsyncdispatchesBuiltIn::HttpActivityas a regular activity, but no worker handles it — resulting inActivityTaskNotFound. This extension fills that gap.What's included
DurableHttpRequest.cs,DurableHttpResponse.cs,HttpRetryOptions.cs,TokenSource.csConverters/HttpMethodConverter.cs,HttpHeadersConverter.cs,TokenSourceConverter.csBuiltInHttpActivity.cs— full retry with exponential backoffDurableTaskBuilderHttpExtensions.cs—UseHttpActivities()TaskOrchestrationContextHttpExtensions.cs— with 202 pollingBuiltInHttpActivityTests.cs,SerializationTests.cs,RegistrationTests.cs(16 tests)Usage
v1 Limitations
TokenSource(managed identity) throwsNotSupportedException— pass tokens via headers insteadIDictionary<string, string>(simplified fromStringValues)Test plan
Closes #696
🤖 Generated with Claude Code