diff --git a/ReleaseNotes.md b/ReleaseNotes.md
index e3df12c17..460241423 100644
--- a/ReleaseNotes.md
+++ b/ReleaseNotes.md
@@ -1,33 +1,20 @@
-## October 2023 (version {0}) aka [Swiss Locomotive](https://en.wikipedia.org/wiki/SBB-CFF-FFS_Ae_6/6) release
-> Codenamed as **[Swiss Locomotive](https://www.google.com/search?q=swiss+locomotive)**
+## Hotfix release (version {0})
+> Default timeout vs the [Quality of Service](https://ocelot.readthedocs.io/en/latest/features/qualityofservice.html) feature
-### Focused On
-
- Logging feature. Performance review, redesign and improvements with new best practices to log
+Special thanks to **Alvin Huang**, @huanguolin!
- - Proposing a centralized `WriteLog` method for the `OcelotLogger`
- - Factory methods for computed strings such as `string.Format` or interpolated strings
- - Using `ILogger.IsEnabled` before calling the native `WriteLog` implementation and invoking string factory method
-
-
- Quality of Service feature. Redesign and stabilization, and it produces less log records now.
-
- - Fixing issue with [Polly](https://www.thepollyproject.org/) Circuit Breaker not opening after max number of retries reached
- - Removing useless log calls that could have an impact on performance
- - Polly [lib](https://www.nuget.org/packages/Polly#versions-body-tab) reference updating to latest `8.2.0` with some code improvements
-
-
- Documentation for Logging, Request ID, Routing and Websockets
-
- - [Logging](https://ocelot.readthedocs.io/en/latest/features/logging.html)
- - [Request ID](https://ocelot.readthedocs.io/en/latest/features/requestid.html)
- - [Routing](https://ocelot.readthedocs.io/en/latest/features/routing.html)
- - [Websockets](https://ocelot.readthedocs.io/en/latest/features/websockets.html)
-
-
- Testing improvements and stabilization aka bug fixing
+### About
+The bug is related to the **Quality of Service** feature (aka **QoS**) and the [HttpClient.Timeout](https://learn.microsoft.com/en-us/dotnet/api/system.net.http.httpclient.timeout) property.
+- If JSON `QoSOptions` section is defined in the route config, then the bug is masked rather than active, and the timeout value is assigned from the [QoS TimeoutValue](https://ocelot.readthedocs.io/en/latest/features/qualityofservice.html#quality-of-service:~:text=%22TimeoutValue%22%3A%205000) property.
+- If the `QoSOptions` section **is not** defined in the route config or the [TimeoutValue](https://ocelot.readthedocs.io/en/latest/features/qualityofservice.html#quality-of-service:~:text=%22TimeoutValue%22%3A%205000) property is missing, then the bug is **active** and affects downstream requests that **never time out**.
- - [Routing](https://ocelot.readthedocs.io/en/latest/features/routing.html) bug fixing: query string placeholders including **CatchAll** one aka `{{everything}}` and query string duplicates removal
- - [QoS](https://ocelot.readthedocs.io/en/latest/features/qualityofservice.html) bug fixing: Polly circuit breaker exceptions
- - Testing bug fixing: rare failed builds because of unstable Polly tests. Acceptance common logic for ports
-
+### Technical info
+In version [22.0](https://github.com/ThreeMammals/Ocelot/releases/tag/22.0.0), the bug was found in the explicit default constructor of the [FileQoSOptions](https://github.com/ThreeMammals/Ocelot/blob/main/src/Ocelot/Configuration/File/FileQoSOptions.cs) class with a maximum [TimeoutValue](https://github.com/ThreeMammals/Ocelot/blob/main/src/Ocelot/Configuration/File/FileQoSOptions.cs#L9). Previously, the default constructor was implicit with the default assignment of zero `0` to all `int` properties.
+
+The new explicit default constructor breaks the old implementation of [QoS TimeoutValue](https://github.com/ThreeMammals/Ocelot/blob/main/src/Ocelot/Requester/HttpClientBuilder.cs#L53-L55) logic, as our [QoS documentation](https://ocelot.readthedocs.io/en/latest/features/qualityofservice.html#quality-of-service:~:text=If%20you%20do%20not%20add%20a%20QoS%20section%2C%20QoS%20will%20not%20be%20used%2C%20however%20Ocelot%20will%20default%20to%20a%2090%20seconds%20timeout%20on%20all%20downstream%20requests.) states:
+![image](https://github.com/ThreeMammals/Ocelot/assets/12430413/2c6b2cd3-e1c6-4510-9e46-883468c140ec)
+**Finally**, the "default 90 second" logic for `HttpClient` breaks down when there are no **QoS** options and all requests on those routes are infinite, if, for example, downstream services are down or stuck.
+
+#### The Bug Artifacts
+- Reported bug issue: [1833](https://github.com/ThreeMammals/Ocelot/issues/1833) by @huanguolin
+- Hotfix PR: [1834](https://github.com/ThreeMammals/Ocelot/pull/1834) by @huanguolin
diff --git a/build.cake b/build.cake
index 1e8f4a032..2b1c99828 100644
--- a/build.cake
+++ b/build.cake
@@ -512,10 +512,10 @@ Task("PublishToNuget")
.Does(() =>
{
Information("Skipping of publishing to NuGet...");
- // if (IsRunningOnCircleCI())
- // {
- // PublishPackages(packagesDir, artifactsFile, nugetFeedStableKey, nugetFeedStableUploadUrl, nugetFeedStableSymbolsUploadUrl);
- // }
+ if (IsRunningOnCircleCI())
+ {
+ PublishPackages(packagesDir, artifactsFile, nugetFeedStableKey, nugetFeedStableUploadUrl, nugetFeedStableSymbolsUploadUrl);
+ }
});
RunTarget(target);
diff --git a/src/Ocelot/Configuration/File/FileQoSOptions.cs b/src/Ocelot/Configuration/File/FileQoSOptions.cs
index 85b51fa43..85282707b 100644
--- a/src/Ocelot/Configuration/File/FileQoSOptions.cs
+++ b/src/Ocelot/Configuration/File/FileQoSOptions.cs
@@ -1,12 +1,19 @@
-namespace Ocelot.Configuration.File
-{
- public class FileQoSOptions
+namespace Ocelot.Configuration.File
+{
+ ///
+ /// File model for the "Quality of Service" feature options of the route.
+ ///
+ public class FileQoSOptions
{
+ ///
+ /// Initializes a new instance of the class.
+ /// Default constructor. DON'T CHANGE!..
+ ///
public FileQoSOptions()
{
DurationOfBreak = 1;
ExceptionsAllowedBeforeBreaking = 0;
- TimeoutValue = int.MaxValue;
+ TimeoutValue = 0;
}
public FileQoSOptions(FileQoSOptions from)
@@ -22,9 +29,9 @@ public FileQoSOptions(QoSOptions from)
ExceptionsAllowedBeforeBreaking = from.ExceptionsAllowedBeforeBreaking;
TimeoutValue = from.TimeoutValue;
}
-
- public int DurationOfBreak { get; set; }
- public int ExceptionsAllowedBeforeBreaking { get; set; }
- public int TimeoutValue { get; set; }
- }
+
+ public int DurationOfBreak { get; set; }
+ public int ExceptionsAllowedBeforeBreaking { get; set; }
+ public int TimeoutValue { get; set; }
+ }
}
diff --git a/test/Ocelot.AcceptanceTests/PollyQoSTests.cs b/test/Ocelot.AcceptanceTests/PollyQoSTests.cs
index f15838169..558ae7e3a 100644
--- a/test/Ocelot.AcceptanceTests/PollyQoSTests.cs
+++ b/test/Ocelot.AcceptanceTests/PollyQoSTests.cs
@@ -15,25 +15,25 @@ public PollyQoSTests()
_steps = new Steps();
}
- private static FileConfiguration FileConfigurationFactory(int port, QoSOptions options,
- string httpMethod = nameof(HttpMethods.Get)) => new()
- {
- Routes = new List
+ private static FileConfiguration FileConfigurationFactory(int port, QoSOptions options, string httpMethod = nameof(HttpMethods.Get))
+ => new()
{
- new()
+ Routes = new List
{
- DownstreamPathTemplate = "/",
- DownstreamScheme = Uri.UriSchemeHttp,
- DownstreamHostAndPorts = new()
+ new()
{
- new("localhost", port),
+ DownstreamPathTemplate = "/",
+ DownstreamScheme = Uri.UriSchemeHttp,
+ DownstreamHostAndPorts = new()
+ {
+ new("localhost", port),
+ },
+ UpstreamPathTemplate = "/",
+ UpstreamHttpMethod = new() {httpMethod},
+ QoSOptions = new FileQoSOptions(options),
},
- UpstreamPathTemplate = "/",
- UpstreamHttpMethod = new() {httpMethod},
- QoSOptions = new FileQoSOptions(options),
},
- },
- };
+ };
[Fact]
public void Should_not_timeout()
@@ -142,7 +142,21 @@ public void Open_circuit_should_not_effect_different_route()
.And(x => _steps.ThenTheResponseBodyShouldBe("Hello from Laura"))
.BDDfy();
}
-
+
+ [Fact(DisplayName = "1833: " + nameof(Should_timeout_per_default_after_90_seconds))]
+ public void Should_timeout_per_default_after_90_seconds()
+ {
+ var port = PortFinder.GetRandomPort();
+ var configuration = FileConfigurationFactory(port, new QoSOptions(new FileQoSOptions()), HttpMethods.Get);
+
+ this.Given(x => x.GivenThereIsAServiceRunningOn($"http://localhost:{port}", 201, string.Empty, 95000))
+ .And(x => _steps.GivenThereIsAConfiguration(configuration))
+ .And(x => _steps.GivenOcelotIsRunningWithPolly())
+ .When(x => _steps.WhenIGetUrlOnTheApiGateway("/"))
+ .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.ServiceUnavailable))
+ .BDDfy();
+ }
+
private static void GivenIWaitMilliseconds(int ms) => Thread.Sleep(ms);
private void GivenThereIsABrokenServiceRunningOn(string url)
diff --git a/test/Ocelot.AcceptanceTests/ReturnsErrorTests.cs b/test/Ocelot.AcceptanceTests/ReturnsErrorTests.cs
index 454b41455..c2157e24d 100644
--- a/test/Ocelot.AcceptanceTests/ReturnsErrorTests.cs
+++ b/test/Ocelot.AcceptanceTests/ReturnsErrorTests.cs
@@ -111,7 +111,7 @@ public void Should_log_warning_if_downstream_service_returns_internal_server_err
.And(x => _steps.GivenThereIsAConfiguration(configuration))
.And(x => _steps.GivenOcelotIsRunningWithLogger())
.When(x => _steps.WhenIGetUrlOnTheApiGateway("/"))
- .Then(x => _steps.ThenWarningShouldBeLogged(2))
+ .Then(x => _steps.ThenWarningShouldBeLogged(1))
.BDDfy();
}
diff --git a/test/Ocelot.UnitTests/Configuration/FileModels/FileQoSOptionsTests.cs b/test/Ocelot.UnitTests/Configuration/FileModels/FileQoSOptionsTests.cs
new file mode 100644
index 000000000..003e1123f
--- /dev/null
+++ b/test/Ocelot.UnitTests/Configuration/FileModels/FileQoSOptionsTests.cs
@@ -0,0 +1,36 @@
+using Ocelot.Configuration.File;
+
+namespace Ocelot.UnitTests.Configuration.FileModels;
+
+public class FileQoSOptionsTests
+{
+ [Fact(DisplayName = "1833: Default constructor must assign zero to the TimeoutValue property")]
+ public void Cstor_Default_AssignedZeroToTimeoutValue()
+ {
+ // Arrange, Act
+ var actual = new FileQoSOptions();
+
+ // Assert
+ Assert.Equal(0, actual.TimeoutValue);
+ }
+
+ [Fact]
+ public void Cstor_Default_AssignedZeroToExceptionsAllowedBeforeBreaking()
+ {
+ // Arrange, Act
+ var actual = new FileQoSOptions();
+
+ // Assert
+ Assert.Equal(0, actual.ExceptionsAllowedBeforeBreaking);
+ }
+
+ [Fact]
+ public void Cstor_Default_AssignedOneToDurationOfBreak()
+ {
+ // Arrange, Act
+ var actual = new FileQoSOptions();
+
+ // Assert
+ Assert.Equal(1, actual.DurationOfBreak);
+ }
+}
diff --git a/test/Ocelot.UnitTests/Requester/HttpClientBuilderTests.cs b/test/Ocelot.UnitTests/Requester/HttpClientBuilderTests.cs
index e352ee4a3..a34b8af19 100644
--- a/test/Ocelot.UnitTests/Requester/HttpClientBuilderTests.cs
+++ b/test/Ocelot.UnitTests/Requester/HttpClientBuilderTests.cs
@@ -11,7 +11,7 @@
namespace Ocelot.UnitTests.Requester
{
- public class HttpClientBuilderTests : IDisposable
+ public sealed class HttpClientBuilderTests : IDisposable
{
private HttpClientBuilder _builder;
private readonly Mock _factory;
@@ -256,6 +256,33 @@ public void should_add_verb_to_cache_key(string verb)
.BDDfy();
}
+ [Theory(DisplayName = "1833: " + nameof(Create_TimeoutValueInQosOptions_HttpClientTimeout))]
+ [InlineData(0, 90)] // default timeout is 90 seconds
+ [InlineData(20, 20)] // QoS timeout
+ public void Create_TimeoutValueInQosOptions_HttpClientTimeout(int qosTimeout, int expectedSeconds)
+ {
+ // Arrange
+ var qosOptions = new QoSOptionsBuilder()
+ .WithTimeoutValue(qosTimeout * 1000)
+ .Build();
+ var handlerOptions = new HttpHandlerOptionsBuilder()
+ .WithUseMaxConnectionPerServer(int.MaxValue)
+ .Build();
+ var route = new DownstreamRouteBuilder()
+ .WithQosOptions(qosOptions)
+ .WithHttpHandlerOptions(handlerOptions)
+ .Build();
+ GivenTheFactoryReturnsNothing();
+
+ // Act
+ var actualClient = _builder.Create(route);
+
+ // Assert
+ var actual = actualClient?.Client?.Timeout;
+ Assert.NotNull(actual);
+ Assert.Equal(expectedSeconds, actual.Value.TotalSeconds);
+ }
+
private void GivenARealCache()
{
_realCache = new MemoryHttpClientCache();