feat(esg): honor per-system TimeoutSeconds in InvokeHttpAsync (falls back to DefaultHttpTimeout)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
This commit is contained in:
@@ -297,12 +297,16 @@ public class ExternalSystemClient : IExternalSystemClient
|
||||
}
|
||||
}
|
||||
|
||||
// Enforce the per-call timeout. ExternalSystemDefinition has no per-system
|
||||
// Timeout field yet, so the configured DefaultHttpTimeout is the effective
|
||||
// round-trip limit (the design's "timeout applies to the HTTP request
|
||||
// round-trip" guarantee). A linked CTS lets us distinguish a timeout from a
|
||||
// caller-initiated cancellation: only the timeout is reclassified as transient.
|
||||
using var timeoutCts = new CancellationTokenSource(_options.DefaultHttpTimeout);
|
||||
// Enforce the per-call timeout (the design's "timeout applies to the HTTP
|
||||
// request round-trip" guarantee). Resolution order: a positive per-system
|
||||
// ExternalSystemDefinition.TimeoutSeconds wins; otherwise the configured
|
||||
// DefaultHttpTimeout is the effective round-trip limit. A linked CTS lets us
|
||||
// distinguish a timeout from a caller-initiated cancellation: only the
|
||||
// timeout is reclassified as transient.
|
||||
var effectiveTimeout = system.TimeoutSeconds > 0
|
||||
? TimeSpan.FromSeconds(system.TimeoutSeconds)
|
||||
: _options.DefaultHttpTimeout;
|
||||
using var timeoutCts = new CancellationTokenSource(effectiveTimeout);
|
||||
using var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(
|
||||
cancellationToken, timeoutCts.Token);
|
||||
|
||||
@@ -320,7 +324,7 @@ public class ExternalSystemClient : IExternalSystemClient
|
||||
{
|
||||
// Our own timeout elapsed — a transient failure per the design.
|
||||
throw ErrorClassifier.AsTransient(
|
||||
$"Timeout calling {system.Name} after {_options.DefaultHttpTimeout.TotalSeconds:0.##}s", ex);
|
||||
$"Timeout calling {system.Name} after {effectiveTimeout.TotalSeconds:0.##}s", ex);
|
||||
}
|
||||
catch (Exception ex) when (ErrorClassifier.IsTransient(ex))
|
||||
{
|
||||
@@ -343,7 +347,7 @@ public class ExternalSystemClient : IExternalSystemClient
|
||||
catch (OperationCanceledException ex) when (timeoutCts.IsCancellationRequested)
|
||||
{
|
||||
throw ErrorClassifier.AsTransient(
|
||||
$"Timeout reading response from {system.Name} after {_options.DefaultHttpTimeout.TotalSeconds:0.##}s", ex);
|
||||
$"Timeout reading response from {system.Name} after {effectiveTimeout.TotalSeconds:0.##}s", ex);
|
||||
}
|
||||
|
||||
if (response.IsSuccessStatusCode)
|
||||
|
||||
@@ -872,6 +872,33 @@ public class ExternalSystemClientTests
|
||||
e.Level >= LogLevel.Warning && e.Message.Contains("TestAPI"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task PerSystemTimeout_OverridesGatewayDefault()
|
||||
{
|
||||
// System timeout 1s; handler black-holes far longer; gateway default is 30s.
|
||||
var system = new ExternalSystemDefinition("slow", "http://x", "none") { Id = 1, TimeoutSeconds = 1 };
|
||||
var method = new ExternalSystemMethod("m", "GET", "/p") { Id = 1, ExternalSystemDefinitionId = 1 };
|
||||
StubResolution(system, method);
|
||||
|
||||
var httpClient = new HttpClient(new HangingHttpMessageHandler(TimeSpan.FromMinutes(10)));
|
||||
_httpClientFactory.CreateClient(Arg.Any<string>()).Returns(httpClient);
|
||||
|
||||
// No options override → DefaultHttpTimeout is the 30s default; only the
|
||||
// per-system 1s should govern the round-trip.
|
||||
var client = new ExternalSystemClient(
|
||||
_httpClientFactory, _repository,
|
||||
NullLogger<ExternalSystemClient>.Instance);
|
||||
|
||||
var sw = System.Diagnostics.Stopwatch.StartNew();
|
||||
var result = await client.CallAsync("slow", "m");
|
||||
sw.Stop();
|
||||
|
||||
Assert.False(result.Success);
|
||||
Assert.Contains("Timeout", result.ErrorMessage);
|
||||
Assert.True(sw.Elapsed < TimeSpan.FromSeconds(4),
|
||||
"per-system 1s timeout must apply, not the 30s default");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Call_TransientFailure_DoesNotLogAtWarningOrAbove()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user