diff --git a/.github/workflows/run-tests-macos-xamarin.yml b/.github/workflows/run-tests-macos-xamarin.yml index 0d2666546..37495a533 100644 --- a/.github/workflows/run-tests-macos-xamarin.yml +++ b/.github/workflows/run-tests-macos-xamarin.yml @@ -21,6 +21,7 @@ jobs: with: dotnet-version: | 3.1.x + 7.0.408 - name: Download dotnet build-script tools run: dotnet tool restore diff --git a/.github/workflows/run-tests-windows-netframework.yml b/.github/workflows/run-tests-windows-netframework.yml index f2ce7acba..758c0bcd1 100644 --- a/.github/workflows/run-tests-windows-netframework.yml +++ b/.github/workflows/run-tests-windows-netframework.yml @@ -16,6 +16,12 @@ jobs: with: submodules: 'recursive' + - name: Download dotnet framework + uses: actions/setup-dotnet@v3 + with: + dotnet-version: | + 7.0.408 + - name: Download dotnet build-script tools run: dotnet tool restore diff --git a/build-script/build-script.fsproj b/build-script/build-script.fsproj index 1eb82ea29..782b4d3e9 100644 --- a/build-script/build-script.fsproj +++ b/build-script/build-script.fsproj @@ -2,7 +2,7 @@ Exe - net6.0 + net7.0 build_script 3390;$(WarnOn) 0020 @@ -12,4 +12,4 @@ - \ No newline at end of file + diff --git a/build-script/paket.references b/build-script/paket.references index 539702f5e..25d73f0b1 100644 --- a/build-script/paket.references +++ b/build-script/paket.references @@ -3,4 +3,4 @@ Microsoft.Build.Tasks.Core Fake.Core.Target Fake.DotNet.AssemblyInfoFile Fake.DotNet.Cli -Fake.DotNet.Testing.XUnit2 \ No newline at end of file +Fake.DotNet.Testing.XUnit2 diff --git a/src/IO.Ably.NETFramework/IO.Ably.NETFramework.csproj b/src/IO.Ably.NETFramework/IO.Ably.NETFramework.csproj index 6c7734c3f..bafefd644 100644 --- a/src/IO.Ably.NETFramework/IO.Ably.NETFramework.csproj +++ b/src/IO.Ably.NETFramework/IO.Ably.NETFramework.csproj @@ -108,4 +108,4 @@ - \ No newline at end of file + diff --git a/src/IO.Ably.Shared/ClientOptions.cs b/src/IO.Ably.Shared/ClientOptions.cs index eba968de6..ed3a546d5 100644 --- a/src/IO.Ably.Shared/ClientOptions.cs +++ b/src/IO.Ably.Shared/ClientOptions.cs @@ -147,10 +147,6 @@ public string[] FallbackHosts internal bool IsProductionEnvironment => Environment.IsEmpty() || Environment.Equals("production", StringComparison.OrdinalIgnoreCase); - internal bool IsDefaultRestHost => FullRestHost() == Defaults.RestHost; - - internal bool IsDefaultRealtimeHost => FullRealtimeHost() == Defaults.RealtimeHost; - internal bool IsDefaultPort => Tls ? TlsPort == Defaults.TlsPort : Port == Defaults.Port; /// @@ -159,20 +155,17 @@ public string[] FallbackHosts /// RestHost. public string FullRestHost() { - var restHost = _restHost; - if (restHost.IsEmpty()) + if (_restHost.IsNotEmpty()) { - restHost = Defaults.RestHost; + return _restHost; } - if (restHost == Defaults.RestHost) + if (!IsProductionEnvironment) { - return IsProductionEnvironment - ? restHost - : $"{Environment}-{restHost}"; + return $"{Environment}-{Defaults.RestHost}"; } - return restHost; + return Defaults.RestHost; } /// @@ -181,27 +174,26 @@ public string FullRestHost() /// RealtimeHost. public string FullRealtimeHost() { - var realtimeHost = _realtimeHost; - if (realtimeHost.IsEmpty()) + if (_realtimeHost.IsNotEmpty()) { - if (_restHost.IsNotEmpty()) - { - Logger.Warning( - $@"restHost is set to {_restHost} but realtimeHost is not set, - so setting realtimeHost to {_restHost} too. If this is not what you want, - please set realtimeHost explicitly."); - return _restHost; - } + return _realtimeHost; + } - realtimeHost = Defaults.RealtimeHost; + if (_restHost.IsNotEmpty()) + { + Logger.Warning( + $@"restHost is set to {_restHost} but realtimeHost is not set, + so setting realtimeHost to {_restHost} too. If this is not what you want, + please set realtimeHost explicitly."); + return _restHost; } - if (realtimeHost == Defaults.RealtimeHost) + if (!IsProductionEnvironment) { - return IsProductionEnvironment ? realtimeHost : $"{Environment}{'-'}{realtimeHost}"; + return $"{Environment}-{Defaults.RealtimeHost}"; } - return realtimeHost; + return Defaults.RealtimeHost; } /// @@ -220,7 +212,7 @@ public string[] GetFallbackHosts() throw new AblyException(new ErrorInfo(msg, ErrorCodes.BadRequest)); } - if (Port != Defaults.Port || TlsPort != Defaults.TlsPort) + if (!IsDefaultPort) { const string msg = "fallbackHostsUseDefault cannot be set when port or tlsPort are set"; throw new AblyException(new ErrorInfo(msg, ErrorCodes.BadRequest)); @@ -230,15 +222,12 @@ public string[] GetFallbackHosts() { Logger.Warning("Deprecated fallbackHostsUseDefault : There is no longer a need to set this when the environment option is also set since the library will now generate the correct fallback hosts using the environment option."); } - else - { - Logger.Warning("Deprecated fallbackHostsUseDefault : fallbackHosts: Ably.Defaults.FALLBACK_HOSTS"); - } + Logger.Warning("Deprecated fallbackHostsUseDefault : fallbackHosts: Ably.Defaults.FALLBACK_HOSTS"); return Defaults.FallbackHosts; } - if (_fallbackHosts is null && _restHost is null && _realtimeHost is null && IsDefaultPort) + if (_fallbackHosts is null && _restHost.IsEmpty() && _realtimeHost.IsEmpty() && IsDefaultPort) { return IsProductionEnvironment ? Defaults.FallbackHosts diff --git a/src/IO.Ably.Shared/Http/AblyHttpClient.cs b/src/IO.Ably.Shared/Http/AblyHttpClient.cs index ea482174d..c94f36658 100644 --- a/src/IO.Ably.Shared/Http/AblyHttpClient.cs +++ b/src/IO.Ably.Shared/Http/AblyHttpClient.cs @@ -52,8 +52,6 @@ internal string RealtimeConnectedFallbackHost private DateTimeOffset? FallbackHostUsedFrom { get; set; } - private bool CanRetry => Options.IsDefaultHost || Options.FallbackHostsUseDefault; - internal void SetPreferredHost(string currentHost) { // If we are retrying the default host we need to clear the preferred one @@ -90,7 +88,7 @@ public async Task Execute(AblyRequest request) int currentTry = 0; var startTime = Now(); - var numberOfRetries = Options.HttpMaxRetryCount; // One for the first request + var maxNumberOfRetries = Options.HttpMaxRetryCount; var host = GetHost(); request.Headers.TryGetValue("request_id", out var requestId); @@ -115,22 +113,20 @@ public async Task Execute(AblyRequest request) break; } - if (CanRetry && response.CanRetry) + if (response.CanRetry) { currentTry++; - Logger.Warning(WrapWithRequestId("Failed response. " + response.GetFailedMessage() + ". Retrying...")); - var (success, newHost) = HandleHostChangeForRetryableFailure(); + var (success, newHost) = HandleHostChangeForRetryableFailure(currentTry); if (success) { Logger.Debug(WrapWithRequestId($"Retrying using host: {newHost}")); - host = newHost; continue; } } - // We only return the request if there is no exception + // We only return the response if there is no exception if (request.NoExceptionOnHttpError && response.Exception == null) { return response.AblyResponse; @@ -140,9 +136,11 @@ public async Task Execute(AblyRequest request) // in that case we need to create a new AblyException throw response.Exception ?? AblyException.FromResponse(response.AblyResponse); } - catch (AblyException) + catch (AblyException ex) { - throw; + var errInfo = ex.ErrorInfo; + errInfo.Message = WrapWithRequestId("Error executing request. " + errInfo.Message); + throw new AblyException(errInfo, ex); } catch (Exception ex) { @@ -150,9 +148,9 @@ public async Task Execute(AblyRequest request) throw new AblyException(new ErrorInfo(WrapWithRequestId("Error executing request. " + ex.Message), ErrorCodes.InternalError), ex); } } - while (currentTry < numberOfRetries); + while (currentTry <= maxNumberOfRetries); // 1 primary host and remaining fallback hosts - throw new AblyException(new ErrorInfo(WrapWithRequestId("Error executing request"), ErrorCodes.InternalError)); + throw new AblyException(new ErrorInfo(WrapWithRequestId("Error executing request, exceeded max no. of retries"), ErrorCodes.InternalError)); List GetFallbackHosts() { @@ -251,7 +249,7 @@ async Task MakeRequest(string requestHost) } } - (bool success, string host) HandleHostChangeForRetryableFailure() + (bool success, string host) HandleHostChangeForRetryableFailure(int attempt) { if (fallbackHosts.Count == 0) { @@ -259,7 +257,7 @@ async Task MakeRequest(string requestHost) return (false, null); } - bool isFirstTryForRequest = currentTry == 1; + bool isFirstTryForRequest = attempt == 1; // If there is a Preferred fallback host already set // and it failed we should try the RealtimeConnected fallback host first diff --git a/src/IO.Ably.Shared/Http/AblyHttpOptions.cs b/src/IO.Ably.Shared/Http/AblyHttpOptions.cs index 05edcb369..69a25d994 100644 --- a/src/IO.Ably.Shared/Http/AblyHttpOptions.cs +++ b/src/IO.Ably.Shared/Http/AblyHttpOptions.cs @@ -25,8 +25,6 @@ internal class AblyHttpOptions internal TimeSpan FallbackRetryTimeOut { get; set; } - public bool IsDefaultHost { get; set; } = true; - internal string[] FallbackHosts { get; set; } internal bool FallbackHostsUseDefault { get; set; } @@ -64,12 +62,11 @@ public AblyHttpOptions(ClientOptions options) IsSecure = options.Tls; Port = options.Tls ? options.TlsPort : options.Port; Host = options.FullRestHost(); - IsDefaultHost = options.IsDefaultRestHost; DisconnectedRetryTimeout = options.DisconnectedRetryTimeout; SuspendedRetryTimeout = options.SuspendedRetryTimeout; HttpOpenTimeout = options.HttpOpenTimeout; HttpRequestTimeout = options.HttpRequestTimeout; - HttpMaxRetryCount = options.IsDefaultRestHost ? options.HttpMaxRetryCount : 1; + HttpMaxRetryCount = options.HttpMaxRetryCount; HttpMaxRetryDuration = options.HttpMaxRetryDuration; FallbackRetryTimeOut = options.FallbackRetryTimeout; FallbackHosts = options.GetFallbackHosts(); diff --git a/src/IO.Ably.Shared/Realtime/AttemptsHelpers.cs b/src/IO.Ably.Shared/Realtime/AttemptsHelpers.cs index 247dff8d6..586441c8a 100644 --- a/src/IO.Ably.Shared/Realtime/AttemptsHelpers.cs +++ b/src/IO.Ably.Shared/Realtime/AttemptsHelpers.cs @@ -9,14 +9,7 @@ internal static class AttemptsHelpers { public static async Task CanFallback(this AblyRest restClient, ErrorInfo error) { - return IsDefaultHost() && - error != null && error.IsRetryableStatusCode() && - await restClient.CanConnectToAbly(); - - bool IsDefaultHost() - { - return restClient.Options.IsDefaultRealtimeHost; - } + return error != null && error.IsRetryableStatusCode() && await restClient.CanConnectToAbly(); } public static bool ShouldSuspend(this RealtimeState state, Func now = null) diff --git a/src/IO.Ably.Tests.NETFramework/IO.Ably.Tests.NETFramework.csproj b/src/IO.Ably.Tests.NETFramework/IO.Ably.Tests.NETFramework.csproj index fa35e1740..ad814c579 100644 --- a/src/IO.Ably.Tests.NETFramework/IO.Ably.Tests.NETFramework.csproj +++ b/src/IO.Ably.Tests.NETFramework/IO.Ably.Tests.NETFramework.csproj @@ -153,4 +153,4 @@ - \ No newline at end of file + diff --git a/src/IO.Ably.Tests.Shared/Infrastructure/TestHelpers.cs b/src/IO.Ably.Tests.Shared/Infrastructure/TestHelpers.cs index c74432700..6e8c292b8 100644 --- a/src/IO.Ably.Tests.Shared/Infrastructure/TestHelpers.cs +++ b/src/IO.Ably.Tests.Shared/Infrastructure/TestHelpers.cs @@ -1,4 +1,6 @@ using System; +using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using IO.Ably.Tests.Infrastructure; using Xunit; @@ -7,6 +9,11 @@ namespace IO.Ably.Tests { internal static class TestHelpers { + public static bool IsSubsetOf(this IEnumerable a, IEnumerable b) + { + return !a.Except(b).Any(); + } + public static void AssertContainsParameter(this AblyRequest request, string key, string value) { Assert.True( diff --git a/src/IO.Ably.Tests.Shared/Realtime/ConnectionAttemptsInfoSpecs.cs b/src/IO.Ably.Tests.Shared/Realtime/ConnectionAttemptsInfoSpecs.cs index 02f7d8e38..5d07db566 100644 --- a/src/IO.Ably.Tests.Shared/Realtime/ConnectionAttemptsInfoSpecs.cs +++ b/src/IO.Ably.Tests.Shared/Realtime/ConnectionAttemptsInfoSpecs.cs @@ -57,15 +57,6 @@ public void ShouldSuspend_WhenFirstAttemptEqualOrGreaterThanConnectionStateTtl_S state.ShouldSuspend(now.ValueFn).Should().BeTrue("When time is greater than"); // > } - [Fact] - public async Task CanAttemptFallback_ShouldBeFalseWithNonDefaultHost() - { - var client = GetRealtime(opts => opts.RealtimeHost = "test.test.com"); - - var result = await client.RestClient.CanFallback(null); - result.Should().BeFalse(); - } - [Theory] [InlineData(500)] [InlineData(501)] @@ -84,7 +75,7 @@ public async Task CanAttemptFallback_WithDefaultHostAndAppropriateError_ShouldBe public async Task CanAttemptFallback_WhenInternetCheckFails_ShouldBeFalse() { var client = GetRealtime(internetCheckOk: false); - var result = await client.RestClient.CanFallback(null); + var result = await client.RestClient.CanFallback(ErrorInfo.ReasonUnknown); result.Should().BeFalse(); } diff --git a/src/IO.Ably.Tests.Shared/Rest/ChannelSandboxSpecs.cs b/src/IO.Ably.Tests.Shared/Rest/ChannelSandboxSpecs.cs index 2bb067a9e..7688292e8 100644 --- a/src/IO.Ably.Tests.Shared/Rest/ChannelSandboxSpecs.cs +++ b/src/IO.Ably.Tests.Shared/Rest/ChannelSandboxSpecs.cs @@ -244,7 +244,6 @@ public async Task IdempotentPublishing_SimulateErrorAndRetry(Protocol protocol) { // setting IsDefaultHost and raising a TaskCanceledException // will cause the request to retry - client.HttpClient.Options.IsDefaultHost = true; throw new TaskCanceledException("faked exception to cause retry"); } @@ -301,7 +300,6 @@ public async Task IdempotentPublishing_WithClientSpecificMessage_ShouldRetry(Pro { // setting IsDefaultHost and raising a TaskCanceledException // will cause the request to retry - client.HttpClient.Options.IsDefaultHost = true; throw new TaskCanceledException("faked exception to cause retry"); } diff --git a/src/IO.Ably.Tests.Shared/Rest/RestSpecs.cs b/src/IO.Ably.Tests.Shared/Rest/RestSpecs.cs index a1cd45dbc..4e02013d9 100644 --- a/src/IO.Ably.Tests.Shared/Rest/RestSpecs.cs +++ b/src/IO.Ably.Tests.Shared/Rest/RestSpecs.cs @@ -490,7 +490,7 @@ public async Task WithRequestIdSet_RequestIdShouldRemainSameIfRetriedToFallbackH client.HttpClient.CreateInternalHttpClient(TimeSpan.FromSeconds(6), handler); var ex = await Assert.ThrowsAsync(() => MakeAnyRequest(client)); - handler.NumberOfRequests.Should().Be(5); + handler.NumberOfRequests.Should().Be(6); // 1 primary host and remaining fallback hosts var uniqueRequestId = handler.LastRequest.Headers.GetValues("request_id").First(); ex.Message.Should().Contain(uniqueRequestId); ex.ErrorInfo.Message.Should().Contain(uniqueRequestId); @@ -523,7 +523,7 @@ public async Task ShouldRetryRequestANumberOfTimes() _ = await Assert.ThrowsAsync(() => MakeAnyRequest(client)); - _handler.NumberOfRequests.Should().Be(client.Options.HttpMaxRetryCount); + _handler.NumberOfRequests.Should().Be(client.Options.HttpMaxRetryCount + 1); // 1 primary host and remaining fallback hosts } [Fact] @@ -604,6 +604,49 @@ public async Task ShouldUseCustomFallbackHostIfProvided() attemptedList[1].Should().Be("www.example.com"); } + [Fact] + [Trait("spec", "RSC15a")] + [Trait("spec", "TO3k6")] + public async Task ShouldUseProvidedCustomFallbackHostsIrrespectiveOfPrimaryHost() + { + _response.StatusCode = HttpStatusCode.BadGateway; + var fallbackHosts = new[] + { + "www.example1.com", + "www.example2.com", + "www.example3.com", + "www.example4.com", + "www.example5.com" + }; + + async Task CheckForAttemptedFallbackHosts(AblyRest client, string primaryHost) + { + var attemptedList = new List(); + _handler.Requests.Clear(); + await Assert.ThrowsAsync(() => MakeAnyRequest(client)); + attemptedList.AddRange(_handler.Requests.Select(x => x.RequestUri.Host).ToList()); + + attemptedList.Count.Should().Be(6); + attemptedList[0].Should().Be(primaryHost); + attemptedList.Skip(1).Should().BeEquivalentTo(fallbackHosts); + } + + var clientWithDefaultPrimaryHost = CreateClient(options => + { + options.FallbackHosts = fallbackHosts; + options.HttpMaxRetryCount = 5; + }); + await CheckForAttemptedFallbackHosts(clientWithDefaultPrimaryHost, "rest.ably.io"); + + var clientWithCustomPrimaryHost = CreateClient(options => + { + options.RestHost = "www.primaryhost.com"; + options.FallbackHosts = fallbackHosts; + options.HttpMaxRetryCount = 5; + }); + await CheckForAttemptedFallbackHosts(clientWithCustomPrimaryHost, "www.primaryhost.com"); + } + [Fact] [Trait("spec", "RSC15a")] [Trait("spec", "TO3k6")] @@ -638,11 +681,9 @@ public async Task ShouldUseDefaultFallbackHostsIfNullArrayProvided() await Assert.ThrowsAsync(() => MakeAnyRequest(client)); attemptedList.AddRange(handler.Requests.Select(x => x.RequestUri.Host).ToList()); - attemptedList.Count.Should().Be(3); // HttpMaxRetryCount defaults to 3 - attemptedList[0].Should().Be("rest.ably.io"); - attemptedList[1].Should().EndWith("ably-realtime.com"); - attemptedList[2].Should().EndWith("ably-realtime.com"); - attemptedList[1].Should().NotBe(attemptedList[2]); + attemptedList.Count.Should().Be(4); // 1 primary host + 3 fallback hosts + attemptedList[0].Should().Be("rest.ably.io"); // primary host + attemptedList.Skip(1).IsSubsetOf(Defaults.FallbackHosts).Should().BeTrue(); } ///