Skip to content

Commit

Permalink
Prepare v9.3.0 release (#96)
Browse files Browse the repository at this point in the history
* When available, include Cloudflare Ray ID into log messages when the HTTP response is not successful

* Fix code formatting

* Bump version

* Improve FetchRequest & FetchResponse public API and make it more future-proof

* Fix bug in UnityWebRequestConfigFetcher

* Add tests to increase code coverage
  • Loading branch information
adams85 authored Aug 28, 2024
1 parent e6e4d77 commit 9d4acd7
Show file tree
Hide file tree
Showing 10 changed files with 229 additions and 55 deletions.
2 changes: 1 addition & 1 deletion appveyor.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
environment:
build_version: 9.2.0
build_version: 9.3.0
version: $(build_version)-{build}
image: Visual Studio 2022
configuration: Release
Expand Down
19 changes: 7 additions & 12 deletions samples/UnityWebGL/Assets/Scripts/SingletonServices.cs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,11 @@ public async Task<FetchResponse> FetchAsync(FetchRequest request, CancellationTo

using var webRequest = UnityWebRequest.Get(uri);

webRequest.SetRequestHeader(request.SdkInfoHeader.Key, request.SdkInfoHeader.Value);
for (int i = 0, n = request.Headers.Count; i < n; i++)
{
var header = request.Headers[i];
webRequest.SetRequestHeader(header.Key, header.Value);
}

webRequest.timeout = (int)request.Timeout.TotalSeconds;

Expand All @@ -177,20 +181,11 @@ public async Task<FetchResponse> FetchAsync(FetchRequest request, CancellationTo

await tcs.Task;

if (webRequest.result == UnityWebRequest.Result.Success)
if (webRequest.result is UnityWebRequest.Result.Success or UnityWebRequest.Result.ProtocolError)
{
var statusCode = (HttpStatusCode)webRequest.responseCode;
Debug.Log($"Fetching config finished with status code {statusCode}.");
if (statusCode == HttpStatusCode.OK)
{
var eTag = webRequest.GetResponseHeader("etag");
var text = webRequest.downloadHandler.text;
return new FetchResponse(statusCode, reasonPhrase: null, eTag is { Length: > 0 } ? eTag : null, webRequest.downloadHandler.text);
}
else
{
return new FetchResponse(statusCode, reasonPhrase: null, null, null);
}
return new FetchResponse(statusCode, reasonPhrase: null, webRequest.GetResponseHeaders(), statusCode == HttpStatusCode.OK ? webRequest.downloadHandler.text : null);
}
else if (webRequest.result == UnityWebRequest.Result.ConnectionError && webRequest.error == "Request timeout")
{
Expand Down
25 changes: 25 additions & 0 deletions src/ConfigCat.Client.Tests/BasicConfigCatClientIntegrationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -475,4 +475,29 @@ public async Task SpecialCharacters_Works(string settingKey, string userId, stri
var actual = await client.GetValueAsync(settingKey, "NOT_CAT", new User(userId));
Assert.AreEqual(expectedValue, actual);
}

[TestMethod]
public async Task ShouldIncludeRayIdInLogMessagesWhenHttpResponseIsNotSuccessful()
{
var logEvents = new List<LogEvent>();
var logger = LoggingHelper.CreateCapturingLogger(logEvents, LogLevel.Info);

using IConfigCatClient client = ConfigCatClient.Get("configcat-sdk-1/~~~~~~~~~~~~~~~~~~~~~~/~~~~~~~~~~~~~~~~~~~~~~", options =>
{
options.PollingMode = PollingModes.ManualPoll;
options.Logger = logger;
});

await client.ForceRefreshAsync();

var errors = logEvents.Where(evt => evt.EventId == 1100).ToArray();
Assert.AreEqual(1, errors.Length);

var error = errors[0].Message;
Assert.AreEqual(1, error.ArgValues.Length);
Assert.IsTrue(error.ArgValues[0] is string);

var rayId = (string)error.ArgValues[0]!;
StringAssert.Contains(errors[0].Message.InvariantFormattedMessage, rayId);
}
}
79 changes: 79 additions & 0 deletions src/ConfigCat.Client.Tests/HttpConfigFetcherTests.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Net;
using System.Net.Http.Headers;
using System.Threading;
using System.Threading.Tasks;
using ConfigCat.Client.Configuration;
using ConfigCat.Client.Tests.Helpers;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Moq;

namespace ConfigCat.Client.Tests;

Expand Down Expand Up @@ -232,4 +237,78 @@ public async Task HttpConfigFetcher_FetchAsync_PendingOperationShouldBeJoined(bo
Assert.IsNull(configFetcher.pendingFetch);
}
}

[TestMethod]
public async Task CustomConfigFetcher_Success()
{
// Arrange

var configJson = File.ReadAllText(Path.Combine(new ConfigLocation.LocalFile("data", "sample_v5.json").GetRealLocation()));

var responseHeader = new[]
{
new KeyValuePair<string, string>("CF-RAY", "CF-12345"),
new KeyValuePair<string, string>("ETag", "\"abc\""),
};

var configFetcherMock = new Mock<IConfigCatConfigFetcher>();
configFetcherMock
.Setup(m => m.FetchAsync(It.IsAny<FetchRequest>(), It.IsAny<CancellationToken>()))
.ReturnsAsync(new FetchResponse(HttpStatusCode.OK, reasonPhrase: null, responseHeader, configJson));

using var client = new ConfigCatClient("test-67890123456789012/1234567890123456789012", new ConfigCatClientOptions
{
ConfigFetcher = configFetcherMock.Object
});

// Act

var value = await client.GetValueAsync("stringDefaultCat", "");

// Assert

Assert.AreEqual("Cat", value);
}

[TestMethod]
public async Task CustomConfigFetcher_Failure()
{
// Arrange

var logEvents = new List<LogEvent>();
var logger = LoggingHelper.CreateCapturingLogger(logEvents, LogLevel.Info);

var responseHeader = new[]
{
new KeyValuePair<string, string>("ETag", "\"abc\""),
new KeyValuePair<string, string>("CF-RAY", "CF-12345"),
};

var configFetcherMock = new Mock<IConfigCatConfigFetcher>();
configFetcherMock
.Setup(m => m.FetchAsync(It.IsAny<FetchRequest>(), It.IsAny<CancellationToken>()))
.ReturnsAsync(new FetchResponse(HttpStatusCode.Forbidden, "Forbidden", responseHeader));

using var client = new ConfigCatClient("test-67890123456789012/1234567890123456789012", new ConfigCatClientOptions
{
ConfigFetcher = configFetcherMock.Object,
Logger = logger.AsWrapper()
});

// Act

await client.ForceRefreshAsync();

// Assert

var errors = logEvents.Where(evt => evt.EventId == 1100).ToArray();
Assert.AreEqual(1, errors.Length);

var error = errors[0].Message;
Assert.AreEqual(1, error.ArgValues.Length);
Assert.IsTrue(error.ArgValues[0] is string);

var rayId = (string)error.ArgValues[0]!;
StringAssert.Contains(errors[0].Message.InvariantFormattedMessage, rayId);
}
}
21 changes: 11 additions & 10 deletions src/ConfigCatClient/ConfigService/DefaultConfigFetcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace ConfigCat.Client;
internal sealed class DefaultConfigFetcher : IConfigFetcher, IDisposable
{
private readonly object syncObj = new();
private readonly KeyValuePair<string, string> sdkInfoHeader;
private readonly IReadOnlyList<KeyValuePair<string, string>> requestHeaders;
private readonly LoggerWrapper logger;
private readonly IConfigCatConfigFetcher configFetcher;
private readonly bool isCustomUri;
Expand All @@ -25,9 +25,10 @@ public DefaultConfigFetcher(Uri requestUri, string productVersion, LoggerWrapper
IConfigCatConfigFetcher configFetcher, bool isCustomUri, TimeSpan timeout)
{
this.requestUri = requestUri;
this.sdkInfoHeader = new KeyValuePair<string, string>(
"X-ConfigCat-UserAgent",
new ProductInfoHeaderValue("ConfigCat-Dotnet", productVersion).ToString());
this.requestHeaders = new[]
{
new KeyValuePair<string, string>("X-ConfigCat-UserAgent", new ProductInfoHeaderValue("ConfigCat-Dotnet", productVersion).ToString())
};
this.logger = logger;
this.configFetcher = configFetcher;
this.isCustomUri = isCustomUri;
Expand Down Expand Up @@ -89,7 +90,7 @@ private async ValueTask<FetchResult> FetchInternalAsync(ProjectConfig lastConfig
if (config is null)
{
var exception = deserializedResponse.Exception;
logMessage = this.logger.FetchReceived200WithInvalidBody(exception);
logMessage = this.logger.FetchReceived200WithInvalidBody(response.RayId, exception);
return FetchResult.Failure(lastConfig, RefreshErrorCode.InvalidHttpResponseContent, logMessage.ToLazyString(), exception);
}

Expand All @@ -104,21 +105,21 @@ private async ValueTask<FetchResult> FetchInternalAsync(ProjectConfig lastConfig
case HttpStatusCode.NotModified:
if (lastConfig.IsEmpty)
{
logMessage = this.logger.FetchReceived304WhenLocalCacheIsEmpty((int)response.StatusCode, response.ReasonPhrase);
logMessage = this.logger.FetchReceived304WhenLocalCacheIsEmpty((int)response.StatusCode, response.ReasonPhrase, response.RayId);
return FetchResult.Failure(lastConfig, RefreshErrorCode.InvalidHttpResponseWhenLocalCacheIsEmpty, logMessage.ToLazyString());
}

return FetchResult.NotModified(lastConfig.With(ProjectConfig.GenerateTimeStamp()));

case HttpStatusCode.Forbidden:
case HttpStatusCode.NotFound:
logMessage = this.logger.FetchFailedDueToInvalidSdkKey();
logMessage = this.logger.FetchFailedDueToInvalidSdkKey(response.RayId);

// We update the timestamp for extra protection against flooding.
return FetchResult.Failure(lastConfig.With(ProjectConfig.GenerateTimeStamp()), RefreshErrorCode.InvalidSdkKey, logMessage.ToLazyString());

default:
logMessage = this.logger.FetchFailedDueToUnexpectedHttpResponse((int)response.StatusCode, response.ReasonPhrase);
logMessage = this.logger.FetchFailedDueToUnexpectedHttpResponse((int)response.StatusCode, response.ReasonPhrase, response.RayId);
return FetchResult.Failure(lastConfig, RefreshErrorCode.UnexpectedHttpResponse, logMessage.ToLazyString());
}
}
Expand Down Expand Up @@ -153,7 +154,7 @@ private async ValueTask<DeserializedResponse> FetchRequestAsync(string? httpETag
{
for (; ; maxExecutionCount--)
{
var request = new FetchRequest(this.requestUri, httpETag, this.sdkInfoHeader, this.timeout);
var request = new FetchRequest(this.requestUri, httpETag, this.requestHeaders, this.timeout);

var response = await this.configFetcher.FetchAsync(request, this.cancellationTokenSource.Token).ConfigureAwait(TaskShim.ContinueOnCapturedContext);

Expand Down Expand Up @@ -193,7 +194,7 @@ private async ValueTask<DeserializedResponse> FetchRequestAsync(string? httpETag

if (maxExecutionCount <= 1)
{
this.logger.FetchFailedDueToRedirectLoop();
this.logger.FetchFailedDueToRedirectLoop(response.RayId);
return new DeserializedResponse(response, config);
}

Expand Down
12 changes: 8 additions & 4 deletions src/ConfigCatClient/ConfigService/FetchRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ public readonly struct FetchRequest
/// <summary>
/// Initializes a new instance of the <see cref="FetchRequest"/> struct.
/// </summary>
public FetchRequest(Uri uri, string? lastETag, KeyValuePair<string, string> sdkInfoHeader, TimeSpan timeout)
public FetchRequest(Uri uri, string? lastETag, IReadOnlyList<KeyValuePair<string, string>> headers, TimeSpan timeout)
{
Uri = uri ?? throw new ArgumentNullException(nameof(uri));
LastETag = lastETag;
SdkInfoHeader = sdkInfoHeader;
Headers = headers;
Timeout = timeout;
}

Expand All @@ -27,13 +27,17 @@ public FetchRequest(Uri uri, string? lastETag, KeyValuePair<string, string> sdkI

/// <summary>
/// The value of the <c>ETag</c> HTTP response header received during the last successful request (if any).
/// If available, should be included in the HTTP request, either in the <c>If-None-Match</c> header or in the <c>ccetag</c> query string parameter.
/// </summary>
/// <remarks>
/// In browser runtime environments the <c>If-None-Match</c> header should be avoided because that may cause unnecessary CORS preflight requests.
/// </remarks>
public string? LastETag { get; }

/// <summary>
/// The name and value of the HTTP request header containing information about the SDK. Should be included in every request.
/// Additional HTTP request headers. Should be included in every HTTP request.
/// </summary>
public KeyValuePair<string, string> SdkInfoHeader { get; }
public IReadOnlyList<KeyValuePair<string, string>> Headers { get; }

/// <summary>
/// The request timeout to apply, configured via <see cref="ConfigCatClientOptions.HttpTimeout"/>.
Expand Down
49 changes: 45 additions & 4 deletions src/ConfigCatClient/ConfigService/FetchResponse.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net;
using System.Net.Http;

namespace ConfigCat.Client;

Expand All @@ -7,15 +11,50 @@ namespace ConfigCat.Client;
/// </summary>
public readonly struct FetchResponse
{
private FetchResponse(HttpStatusCode statusCode, string? reasonPhrase, string? body)
{
StatusCode = statusCode;
ReasonPhrase = reasonPhrase;
Body = body;
}

internal FetchResponse(HttpResponseMessage httpResponse, string? httpResponseBody = null)
: this(httpResponse.StatusCode, httpResponse.ReasonPhrase, httpResponseBody)
{
ETag = httpResponse.Headers.ETag?.Tag;
RayId = httpResponse.Headers.TryGetValues("CF-RAY", out var values) ? values.FirstOrDefault() : null;
}

/// <summary>
/// Initializes a new instance of the <see cref="FetchResponse"/> struct.
/// </summary>
public FetchResponse(HttpStatusCode statusCode, string? reasonPhrase, string? eTag, string? body)
public FetchResponse(HttpStatusCode statusCode, string? reasonPhrase, IEnumerable<KeyValuePair<string, string>> headers, string? body = null)
: this(statusCode, reasonPhrase, body)
{
StatusCode = statusCode;
ReasonPhrase = reasonPhrase;
string? eTag = null, rayId = null;

foreach (var header in headers)
{
if (eTag is null && "ETag".Equals(header.Key, StringComparison.OrdinalIgnoreCase))
{
eTag = header.Value;
if (rayId is not null)
{
break;
}
}
else if (rayId is null && "CF-RAY".Equals(header.Key, StringComparison.OrdinalIgnoreCase))
{
rayId = header.Value;
if (eTag is not null)
{
break;
}
}
}

ETag = eTag;
Body = body;
RayId = rayId;
}

/// <summary>
Expand All @@ -33,6 +72,8 @@ public FetchResponse(HttpStatusCode statusCode, string? reasonPhrase, string? eT
/// </summary>
public string? ETag { get; }

internal string? RayId { get; }

/// <summary>
/// The response body.
/// </summary>
Expand Down
10 changes: 7 additions & 3 deletions src/ConfigCatClient/ConfigService/HttpClientConfigFetcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ public async Task<FetchResponse> FetchAsync(FetchRequest request, CancellationTo
RequestUri = request.Uri,
};

httpRequest.Headers.Add(request.SdkInfoHeader.Key, request.SdkInfoHeader.Value);
for (int i = 0, n = request.Headers.Count; i < n; i++)
{
var header = request.Headers[i];
httpRequest.Headers.Add(header.Key, header.Value);
}

if (request.LastETag is not null)
{
Expand All @@ -90,11 +94,11 @@ public async Task<FetchResponse> FetchAsync(FetchRequest request, CancellationTo
var httpResponseBody = await httpResponse.Content.ReadAsStringAsync().ConfigureAwait(TaskShim.ContinueOnCapturedContext);
#endif

return new FetchResponse(httpResponse.StatusCode, httpResponse.ReasonPhrase, httpResponse.Headers.ETag?.Tag, httpResponseBody);
return new FetchResponse(httpResponse, httpResponseBody);
}
else
{
var response = new FetchResponse(httpResponse.StatusCode, httpResponse.ReasonPhrase, eTag: null, body: null);
var response = new FetchResponse(httpResponse);
if (!response.IsExpected)
{
this.httpClient = null;
Expand Down
Loading

0 comments on commit 9d4acd7

Please sign in to comment.