From df43e4b1b40dd85afe7a7120bb06d980da86b850 Mon Sep 17 00:00:00 2001 From: Adam Simon Date: Thu, 22 Aug 2024 17:13:33 +0200 Subject: [PATCH 1/6] Include Cloudflare Ray ID into log messages when the HTTP response is not successful --- .../BasicConfigCatClientIntegrationTests.cs | 25 ++++++++++++++++ .../ConfigService/DefaultConfigFetcher.cs | 10 +++---- .../ConfigService/FetchResponse.cs | 27 +++++++++++++---- .../ConfigService/HttpClientConfigFetcher.cs | 4 +-- src/ConfigCatClient/Logging/LogMessages.cs | 29 +++++++++++-------- 5 files changed, 70 insertions(+), 25 deletions(-) diff --git a/src/ConfigCat.Client.Tests/BasicConfigCatClientIntegrationTests.cs b/src/ConfigCat.Client.Tests/BasicConfigCatClientIntegrationTests.cs index 7dd29dad..50f88840 100644 --- a/src/ConfigCat.Client.Tests/BasicConfigCatClientIntegrationTests.cs +++ b/src/ConfigCat.Client.Tests/BasicConfigCatClientIntegrationTests.cs @@ -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(); + 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 rayId = errors[0].Message.ArgValues[0] as string; + Assert.IsNotNull(rayId); + Assert.AreNotEqual("", rayId); + Assert.AreNotEqual(LoggerExtensions.FormatRayId(null), rayId); + + StringAssert.Contains(errors[0].Message.InvariantFormattedMessage, rayId); + } } diff --git a/src/ConfigCatClient/ConfigService/DefaultConfigFetcher.cs b/src/ConfigCatClient/ConfigService/DefaultConfigFetcher.cs index d503ca5f..d3ba178c 100644 --- a/src/ConfigCatClient/ConfigService/DefaultConfigFetcher.cs +++ b/src/ConfigCatClient/ConfigService/DefaultConfigFetcher.cs @@ -89,7 +89,7 @@ private async ValueTask 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); } @@ -104,7 +104,7 @@ private async ValueTask 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()); } @@ -112,13 +112,13 @@ private async ValueTask FetchInternalAsync(ProjectConfig lastConfig 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()); } } @@ -193,7 +193,7 @@ private async ValueTask FetchRequestAsync(string? httpETag if (maxExecutionCount <= 1) { - this.logger.FetchFailedDueToRedirectLoop(); + this.logger.FetchFailedDueToRedirectLoop(response.RayId); return new DeserializedResponse(response, config); } diff --git a/src/ConfigCatClient/ConfigService/FetchResponse.cs b/src/ConfigCatClient/ConfigService/FetchResponse.cs index d1907a1b..1ba381f3 100644 --- a/src/ConfigCatClient/ConfigService/FetchResponse.cs +++ b/src/ConfigCatClient/ConfigService/FetchResponse.cs @@ -1,4 +1,7 @@ +using System.Linq; using System.Net; +using System.Net.Http; +using System.Net.Http.Headers; namespace ConfigCat.Client; @@ -7,17 +10,27 @@ namespace ConfigCat.Client; /// public readonly struct FetchResponse { - /// - /// Initializes a new instance of the struct. - /// - public FetchResponse(HttpStatusCode statusCode, string? reasonPhrase, string? eTag, string? body) + internal static FetchResponse From(HttpResponseMessage httpResponse, string? httpResponseBody = null) + { + return new FetchResponse(httpResponse.StatusCode, httpResponse.ReasonPhrase, httpResponse.Headers, httpResponseBody); + } + + private readonly object? headersOrETag; // either null or a string or HttpResponseHeaders + + private FetchResponse(HttpStatusCode statusCode, string? reasonPhrase, object? headersOrETag, string? body) { StatusCode = statusCode; ReasonPhrase = reasonPhrase; - ETag = eTag; + this.headersOrETag = headersOrETag; Body = body; } + /// + /// Initializes a new instance of the struct. + /// + public FetchResponse(HttpStatusCode statusCode, string? reasonPhrase, string? eTag, string? body) + : this(statusCode, reasonPhrase, (object?)eTag, body) { } + /// /// The HTTP status code. /// @@ -31,7 +44,9 @@ public FetchResponse(HttpStatusCode statusCode, string? reasonPhrase, string? eT /// /// The value of the ETag HTTP response header. /// - public string? ETag { get; } + public string? ETag => this.headersOrETag is HttpResponseHeaders headers ? headers.ETag?.Tag : (string?)this.headersOrETag; + + internal string? RayId => this.headersOrETag is HttpResponseHeaders headers && headers.TryGetValues("CF-RAY", out var values) ? values.FirstOrDefault() : null; /// /// The response body. diff --git a/src/ConfigCatClient/ConfigService/HttpClientConfigFetcher.cs b/src/ConfigCatClient/ConfigService/HttpClientConfigFetcher.cs index 546d6bf3..517f0883 100644 --- a/src/ConfigCatClient/ConfigService/HttpClientConfigFetcher.cs +++ b/src/ConfigCatClient/ConfigService/HttpClientConfigFetcher.cs @@ -90,11 +90,11 @@ public async Task 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 FetchResponse.From(httpResponse, httpResponseBody); } else { - var response = new FetchResponse(httpResponse.StatusCode, httpResponse.ReasonPhrase, eTag: null, body: null); + var response = FetchResponse.From(httpResponse); if (!response.IsExpected) { this.httpClient = null; diff --git a/src/ConfigCatClient/Logging/LogMessages.cs b/src/ConfigCatClient/Logging/LogMessages.cs index 433e40fb..12a6fbdb 100644 --- a/src/ConfigCatClient/Logging/LogMessages.cs +++ b/src/ConfigCatClient/Logging/LogMessages.cs @@ -38,14 +38,15 @@ public static FormattableLogMessage ForceRefreshError(this LoggerWrapper logger, $"Error occurred in the `{methodName}` method.", "METHOD_NAME"); - public static FormattableLogMessage FetchFailedDueToInvalidSdkKey(this LoggerWrapper logger) => logger.Log( + public static FormattableLogMessage FetchFailedDueToInvalidSdkKey(this LoggerWrapper logger, string? rayId) => logger.LogInterpolated( LogLevel.Error, 1100, - "Your SDK Key seems to be wrong. You can find the valid SDK Key at https://app.configcat.com/sdkkey"); + $"Your SDK Key seems to be wrong. You can find the valid SDK Key at https://app.configcat.com/sdkkey (Ray ID: {FormatRayId(rayId)})", + "RAY_ID"); - public static FormattableLogMessage FetchFailedDueToUnexpectedHttpResponse(this LoggerWrapper logger, int statusCode, string? reasonPhrase) => logger.LogInterpolated( + public static FormattableLogMessage FetchFailedDueToUnexpectedHttpResponse(this LoggerWrapper logger, int statusCode, string? reasonPhrase, string? rayId) => logger.LogInterpolated( LogLevel.Error, 1101, - $"Unexpected HTTP response was received while trying to fetch config JSON: {statusCode} {reasonPhrase}", - "STATUS_CODE", "REASON_PHRASE"); + $"Unexpected HTTP response was received while trying to fetch config JSON: {statusCode} {reasonPhrase} (Ray ID: {FormatRayId(rayId)})", + "STATUS_CODE", "REASON_PHRASE", "RAY_ID"); public static FormattableLogMessage FetchFailedDueToRequestTimeout(this LoggerWrapper logger, TimeSpan timeout, Exception ex) => logger.LogInterpolated( LogLevel.Error, 1102, ex, @@ -56,18 +57,20 @@ public static FormattableLogMessage FetchFailedDueToUnexpectedError(this LoggerW LogLevel.Error, 1103, ex, "Unexpected error occurred while trying to fetch config JSON. It is most likely due to a local network issue. Please make sure your application can reach the ConfigCat CDN servers (or your proxy server) over HTTP."); - public static FormattableLogMessage FetchFailedDueToRedirectLoop(this LoggerWrapper logger) => logger.Log( + public static FormattableLogMessage FetchFailedDueToRedirectLoop(this LoggerWrapper logger, string? rayId) => logger.LogInterpolated( LogLevel.Error, 1104, - "Redirection loop encountered while trying to fetch config JSON. Please contact us at https://configcat.com/support/"); + $"Redirection loop encountered while trying to fetch config JSON. Please contact us at https://configcat.com/support/ (Ray ID: {FormatRayId(rayId)})", + "RAY_ID"); - public static FormattableLogMessage FetchReceived200WithInvalidBody(this LoggerWrapper logger, Exception? ex) => logger.Log( + public static FormattableLogMessage FetchReceived200WithInvalidBody(this LoggerWrapper logger, string? rayId, Exception? ex) => logger.LogInterpolated( LogLevel.Error, 1105, ex, - "Fetching config JSON was successful but the HTTP response content was invalid."); + $"Fetching config JSON was successful but the HTTP response content was invalid. (Ray ID: {FormatRayId(rayId)})", + "RAY_ID"); - public static FormattableLogMessage FetchReceived304WhenLocalCacheIsEmpty(this LoggerWrapper logger, int statusCode, string? reasonPhrase) => logger.LogInterpolated( + public static FormattableLogMessage FetchReceived304WhenLocalCacheIsEmpty(this LoggerWrapper logger, int statusCode, string? reasonPhrase, string? rayId) => logger.LogInterpolated( LogLevel.Error, 1106, - $"Unexpected HTTP response was received when no config JSON is cached locally: {statusCode} {reasonPhrase}", - "STATUS_CODE", "REASON_PHRASE"); + $"Unexpected HTTP response was received when no config JSON is cached locally: {statusCode} {reasonPhrase} (Ray ID: {FormatRayId(rayId)})", + "STATUS_CODE", "REASON_PHRASE", "RAY_ID"); public static FormattableLogMessage AutoPollConfigServiceErrorDuringPolling(this LoggerWrapper logger, Exception ex) => logger.Log( LogLevel.Error, 1200, ex, @@ -194,4 +197,6 @@ public static FormattableLogMessage LocalFileDataSourceReloadsFile(this LoggerWr #region SDK-specific info messages (6000-6999) #endregion + + internal static string FormatRayId(string? value) => value ?? "n/a"; } From 898f31bd4c50291caaa0317d2a1ca136d9b21007 Mon Sep 17 00:00:00 2001 From: Adam Simon Date: Thu, 22 Aug 2024 17:15:33 +0200 Subject: [PATCH 2/6] Fix code formatting --- src/ConfigCatClient/Utils/SerializationHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ConfigCatClient/Utils/SerializationHelper.cs b/src/ConfigCatClient/Utils/SerializationHelper.cs index 3394f265..bb36e963 100644 --- a/src/ConfigCatClient/Utils/SerializationHelper.cs +++ b/src/ConfigCatClient/Utils/SerializationHelper.cs @@ -255,4 +255,4 @@ private static string UnescapeAstralCodePoints(string json) }); } #endif - } +} From 28fc0af91d9e98febffdb8333905437653a84208 Mon Sep 17 00:00:00 2001 From: Adam Simon Date: Thu, 22 Aug 2024 17:14:06 +0200 Subject: [PATCH 3/6] Bump version --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 8666becb..12cd852c 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -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 From 9e93e99025d898243a355db4060e6d251401e32d Mon Sep 17 00:00:00 2001 From: Adam Simon Date: Thu, 22 Aug 2024 17:47:52 +0200 Subject: [PATCH 4/6] Include Ray ID in log messages only when available --- .../BasicConfigCatClientIntegrationTests.cs | 8 +-- src/ConfigCatClient/Logging/LogMessages.cs | 70 ++++++++++++------- 2 files changed, 49 insertions(+), 29 deletions(-) diff --git a/src/ConfigCat.Client.Tests/BasicConfigCatClientIntegrationTests.cs b/src/ConfigCat.Client.Tests/BasicConfigCatClientIntegrationTests.cs index 50f88840..6fba615d 100644 --- a/src/ConfigCat.Client.Tests/BasicConfigCatClientIntegrationTests.cs +++ b/src/ConfigCat.Client.Tests/BasicConfigCatClientIntegrationTests.cs @@ -493,11 +493,11 @@ public async Task ShouldIncludeRayIdInLogMessagesWhenHttpResponseIsNotSuccessful var errors = logEvents.Where(evt => evt.EventId == 1100).ToArray(); Assert.AreEqual(1, errors.Length); - var rayId = errors[0].Message.ArgValues[0] as string; - Assert.IsNotNull(rayId); - Assert.AreNotEqual("", rayId); - Assert.AreNotEqual(LoggerExtensions.FormatRayId(null), rayId); + 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); } } diff --git a/src/ConfigCatClient/Logging/LogMessages.cs b/src/ConfigCatClient/Logging/LogMessages.cs index 12a6fbdb..0b8f67cf 100644 --- a/src/ConfigCatClient/Logging/LogMessages.cs +++ b/src/ConfigCatClient/Logging/LogMessages.cs @@ -38,15 +38,24 @@ public static FormattableLogMessage ForceRefreshError(this LoggerWrapper logger, $"Error occurred in the `{methodName}` method.", "METHOD_NAME"); - public static FormattableLogMessage FetchFailedDueToInvalidSdkKey(this LoggerWrapper logger, string? rayId) => logger.LogInterpolated( - LogLevel.Error, 1100, - $"Your SDK Key seems to be wrong. You can find the valid SDK Key at https://app.configcat.com/sdkkey (Ray ID: {FormatRayId(rayId)})", - "RAY_ID"); - - public static FormattableLogMessage FetchFailedDueToUnexpectedHttpResponse(this LoggerWrapper logger, int statusCode, string? reasonPhrase, string? rayId) => logger.LogInterpolated( - LogLevel.Error, 1101, - $"Unexpected HTTP response was received while trying to fetch config JSON: {statusCode} {reasonPhrase} (Ray ID: {FormatRayId(rayId)})", - "STATUS_CODE", "REASON_PHRASE", "RAY_ID"); + public static FormattableLogMessage FetchFailedDueToInvalidSdkKey(this LoggerWrapper logger, string? rayId) => rayId is not null + ? logger.LogInterpolated( + LogLevel.Error, 1100, + $"Your SDK Key seems to be wrong. You can find the valid SDK Key at https://app.configcat.com/sdkkey (Ray ID: {rayId})", + "RAY_ID") + : logger.Log( + LogLevel.Error, 1100, + "Your SDK Key seems to be wrong. You can find the valid SDK Key at https://app.configcat.com/sdkkey"); + + public static FormattableLogMessage FetchFailedDueToUnexpectedHttpResponse(this LoggerWrapper logger, int statusCode, string? reasonPhrase, string? rayId) => rayId is not null + ? logger.LogInterpolated( + LogLevel.Error, 1101, + $"Unexpected HTTP response was received while trying to fetch config JSON: {statusCode} {reasonPhrase} (Ray ID: {rayId})", + "STATUS_CODE", "REASON_PHRASE", "RAY_ID") + : logger.LogInterpolated( + LogLevel.Error, 1101, + $"Unexpected HTTP response was received while trying to fetch config JSON: {statusCode} {reasonPhrase}", + "STATUS_CODE", "REASON_PHRASE"); public static FormattableLogMessage FetchFailedDueToRequestTimeout(this LoggerWrapper logger, TimeSpan timeout, Exception ex) => logger.LogInterpolated( LogLevel.Error, 1102, ex, @@ -57,20 +66,33 @@ public static FormattableLogMessage FetchFailedDueToUnexpectedError(this LoggerW LogLevel.Error, 1103, ex, "Unexpected error occurred while trying to fetch config JSON. It is most likely due to a local network issue. Please make sure your application can reach the ConfigCat CDN servers (or your proxy server) over HTTP."); - public static FormattableLogMessage FetchFailedDueToRedirectLoop(this LoggerWrapper logger, string? rayId) => logger.LogInterpolated( - LogLevel.Error, 1104, - $"Redirection loop encountered while trying to fetch config JSON. Please contact us at https://configcat.com/support/ (Ray ID: {FormatRayId(rayId)})", - "RAY_ID"); - - public static FormattableLogMessage FetchReceived200WithInvalidBody(this LoggerWrapper logger, string? rayId, Exception? ex) => logger.LogInterpolated( - LogLevel.Error, 1105, ex, - $"Fetching config JSON was successful but the HTTP response content was invalid. (Ray ID: {FormatRayId(rayId)})", - "RAY_ID"); - - public static FormattableLogMessage FetchReceived304WhenLocalCacheIsEmpty(this LoggerWrapper logger, int statusCode, string? reasonPhrase, string? rayId) => logger.LogInterpolated( - LogLevel.Error, 1106, - $"Unexpected HTTP response was received when no config JSON is cached locally: {statusCode} {reasonPhrase} (Ray ID: {FormatRayId(rayId)})", - "STATUS_CODE", "REASON_PHRASE", "RAY_ID"); + public static FormattableLogMessage FetchFailedDueToRedirectLoop(this LoggerWrapper logger, string? rayId) => rayId is not null + ? logger.LogInterpolated( + LogLevel.Error, 1104, + $"Redirection loop encountered while trying to fetch config JSON. Please contact us at https://configcat.com/support/ (Ray ID: {rayId})", + "RAY_ID") + : logger.Log( + LogLevel.Error, 1104, + "Redirection loop encountered while trying to fetch config JSON. Please contact us at https://configcat.com/support/"); + + public static FormattableLogMessage FetchReceived200WithInvalidBody(this LoggerWrapper logger, string? rayId, Exception? ex) => rayId is not null + ? logger.LogInterpolated( + LogLevel.Error, 1105, ex, + $"Fetching config JSON was successful but the HTTP response content was invalid. (Ray ID: {rayId})", + "RAY_ID") + : logger.Log( + LogLevel.Error, 1105, ex, + "Fetching config JSON was successful but the HTTP response content was invalid."); + + public static FormattableLogMessage FetchReceived304WhenLocalCacheIsEmpty(this LoggerWrapper logger, int statusCode, string? reasonPhrase, string? rayId) => rayId is not null + ? logger.LogInterpolated( + LogLevel.Error, 1106, + $"Unexpected HTTP response was received when no config JSON is cached locally: {statusCode} {reasonPhrase} (Ray ID: {rayId})", + "STATUS_CODE", "REASON_PHRASE", "RAY_ID") + : logger.LogInterpolated( + LogLevel.Error, 1106, + $"Unexpected HTTP response was received when no config JSON is cached locally: {statusCode} {reasonPhrase}", + "STATUS_CODE", "REASON_PHRASE"); public static FormattableLogMessage AutoPollConfigServiceErrorDuringPolling(this LoggerWrapper logger, Exception ex) => logger.Log( LogLevel.Error, 1200, ex, @@ -197,6 +219,4 @@ public static FormattableLogMessage LocalFileDataSourceReloadsFile(this LoggerWr #region SDK-specific info messages (6000-6999) #endregion - - internal static string FormatRayId(string? value) => value ?? "n/a"; } From 96c1aa16cc7c28abe450bf2145e94966d82a173a Mon Sep 17 00:00:00 2001 From: Adam Simon Date: Tue, 27 Aug 2024 09:45:59 +0200 Subject: [PATCH 5/6] Improve FetchRequest & FetchResponse public API and make it more future-proof + fix bug in UnityWebRequestConfigFetcher --- .../Assets/Scripts/SingletonServices.cs | 19 +++---- .../ConfigService/DefaultConfigFetcher.cs | 11 ++-- .../ConfigService/FetchRequest.cs | 12 +++-- .../ConfigService/FetchResponse.cs | 54 ++++++++++++++----- .../ConfigService/HttpClientConfigFetcher.cs | 10 ++-- 5 files changed, 68 insertions(+), 38 deletions(-) diff --git a/samples/UnityWebGL/Assets/Scripts/SingletonServices.cs b/samples/UnityWebGL/Assets/Scripts/SingletonServices.cs index f55142c0..4ddb9113 100644 --- a/samples/UnityWebGL/Assets/Scripts/SingletonServices.cs +++ b/samples/UnityWebGL/Assets/Scripts/SingletonServices.cs @@ -153,7 +153,11 @@ public async Task 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; @@ -177,20 +181,11 @@ public async Task 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") { diff --git a/src/ConfigCatClient/ConfigService/DefaultConfigFetcher.cs b/src/ConfigCatClient/ConfigService/DefaultConfigFetcher.cs index d3ba178c..00fab451 100644 --- a/src/ConfigCatClient/ConfigService/DefaultConfigFetcher.cs +++ b/src/ConfigCatClient/ConfigService/DefaultConfigFetcher.cs @@ -11,7 +11,7 @@ namespace ConfigCat.Client; internal sealed class DefaultConfigFetcher : IConfigFetcher, IDisposable { private readonly object syncObj = new(); - private readonly KeyValuePair sdkInfoHeader; + private readonly IReadOnlyList> requestHeaders; private readonly LoggerWrapper logger; private readonly IConfigCatConfigFetcher configFetcher; private readonly bool isCustomUri; @@ -25,9 +25,10 @@ public DefaultConfigFetcher(Uri requestUri, string productVersion, LoggerWrapper IConfigCatConfigFetcher configFetcher, bool isCustomUri, TimeSpan timeout) { this.requestUri = requestUri; - this.sdkInfoHeader = new KeyValuePair( - "X-ConfigCat-UserAgent", - new ProductInfoHeaderValue("ConfigCat-Dotnet", productVersion).ToString()); + this.requestHeaders = new[] + { + new KeyValuePair("X-ConfigCat-UserAgent", new ProductInfoHeaderValue("ConfigCat-Dotnet", productVersion).ToString()) + }; this.logger = logger; this.configFetcher = configFetcher; this.isCustomUri = isCustomUri; @@ -153,7 +154,7 @@ private async ValueTask 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); diff --git a/src/ConfigCatClient/ConfigService/FetchRequest.cs b/src/ConfigCatClient/ConfigService/FetchRequest.cs index 2590cb99..3591445b 100644 --- a/src/ConfigCatClient/ConfigService/FetchRequest.cs +++ b/src/ConfigCatClient/ConfigService/FetchRequest.cs @@ -12,11 +12,11 @@ public readonly struct FetchRequest /// /// Initializes a new instance of the struct. /// - public FetchRequest(Uri uri, string? lastETag, KeyValuePair sdkInfoHeader, TimeSpan timeout) + public FetchRequest(Uri uri, string? lastETag, IReadOnlyList> headers, TimeSpan timeout) { Uri = uri ?? throw new ArgumentNullException(nameof(uri)); LastETag = lastETag; - SdkInfoHeader = sdkInfoHeader; + Headers = headers; Timeout = timeout; } @@ -27,13 +27,17 @@ public FetchRequest(Uri uri, string? lastETag, KeyValuePair sdkI /// /// The value of the ETag HTTP response header received during the last successful request (if any). + /// If available, should be included in the HTTP request, either in the If-None-Match header or in the ccetag query string parameter. /// + /// + /// In browser runtime environments the If-None-Match header should be avoided because that may cause unnecessary CORS preflight requests. + /// public string? LastETag { get; } /// - /// 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. /// - public KeyValuePair SdkInfoHeader { get; } + public IReadOnlyList> Headers { get; } /// /// The request timeout to apply, configured via . diff --git a/src/ConfigCatClient/ConfigService/FetchResponse.cs b/src/ConfigCatClient/ConfigService/FetchResponse.cs index 1ba381f3..1bc43aa2 100644 --- a/src/ConfigCatClient/ConfigService/FetchResponse.cs +++ b/src/ConfigCatClient/ConfigService/FetchResponse.cs @@ -1,7 +1,8 @@ +using System; +using System.Collections.Generic; using System.Linq; using System.Net; using System.Net.Http; -using System.Net.Http.Headers; namespace ConfigCat.Client; @@ -10,26 +11,51 @@ namespace ConfigCat.Client; /// public readonly struct FetchResponse { - internal static FetchResponse From(HttpResponseMessage httpResponse, string? httpResponseBody = null) - { - return new FetchResponse(httpResponse.StatusCode, httpResponse.ReasonPhrase, httpResponse.Headers, httpResponseBody); - } - - private readonly object? headersOrETag; // either null or a string or HttpResponseHeaders - - private FetchResponse(HttpStatusCode statusCode, string? reasonPhrase, object? headersOrETag, string? body) + private FetchResponse(HttpStatusCode statusCode, string? reasonPhrase, string? body) { StatusCode = statusCode; ReasonPhrase = reasonPhrase; - this.headersOrETag = headersOrETag; 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; + } + /// /// Initializes a new instance of the struct. /// - public FetchResponse(HttpStatusCode statusCode, string? reasonPhrase, string? eTag, string? body) - : this(statusCode, reasonPhrase, (object?)eTag, body) { } + public FetchResponse(HttpStatusCode statusCode, string? reasonPhrase, IEnumerable> headers, string? body = null) + : this(statusCode, reasonPhrase, body) + { + 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; + RayId = rayId; + } /// /// The HTTP status code. @@ -44,9 +70,9 @@ public FetchResponse(HttpStatusCode statusCode, string? reasonPhrase, string? eT /// /// The value of the ETag HTTP response header. /// - public string? ETag => this.headersOrETag is HttpResponseHeaders headers ? headers.ETag?.Tag : (string?)this.headersOrETag; + public string? ETag { get; } - internal string? RayId => this.headersOrETag is HttpResponseHeaders headers && headers.TryGetValues("CF-RAY", out var values) ? values.FirstOrDefault() : null; + internal string? RayId { get; } /// /// The response body. diff --git a/src/ConfigCatClient/ConfigService/HttpClientConfigFetcher.cs b/src/ConfigCatClient/ConfigService/HttpClientConfigFetcher.cs index 517f0883..edca7183 100644 --- a/src/ConfigCatClient/ConfigService/HttpClientConfigFetcher.cs +++ b/src/ConfigCatClient/ConfigService/HttpClientConfigFetcher.cs @@ -71,7 +71,11 @@ public async Task 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) { @@ -90,11 +94,11 @@ public async Task FetchAsync(FetchRequest request, CancellationTo var httpResponseBody = await httpResponse.Content.ReadAsStringAsync().ConfigureAwait(TaskShim.ContinueOnCapturedContext); #endif - return FetchResponse.From(httpResponse, httpResponseBody); + return new FetchResponse(httpResponse, httpResponseBody); } else { - var response = FetchResponse.From(httpResponse); + var response = new FetchResponse(httpResponse); if (!response.IsExpected) { this.httpClient = null; From b3f6d4c4b46621924cfec984095052bc9b39d009 Mon Sep 17 00:00:00 2001 From: Adam Simon Date: Tue, 27 Aug 2024 10:58:47 +0200 Subject: [PATCH 6/6] Add tests to increase code coverage --- .../HttpConfigFetcherTests.cs | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/src/ConfigCat.Client.Tests/HttpConfigFetcherTests.cs b/src/ConfigCat.Client.Tests/HttpConfigFetcherTests.cs index a223e0fc..225c754b 100644 --- a/src/ConfigCat.Client.Tests/HttpConfigFetcherTests.cs +++ b/src/ConfigCat.Client.Tests/HttpConfigFetcherTests.cs @@ -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; @@ -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("CF-RAY", "CF-12345"), + new KeyValuePair("ETag", "\"abc\""), + }; + + var configFetcherMock = new Mock(); + configFetcherMock + .Setup(m => m.FetchAsync(It.IsAny(), It.IsAny())) + .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(); + var logger = LoggingHelper.CreateCapturingLogger(logEvents, LogLevel.Info); + + var responseHeader = new[] + { + new KeyValuePair("ETag", "\"abc\""), + new KeyValuePair("CF-RAY", "CF-12345"), + }; + + var configFetcherMock = new Mock(); + configFetcherMock + .Setup(m => m.FetchAsync(It.IsAny(), It.IsAny())) + .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); + } }