Skip to content

Commit b4e9886

Browse files
marandanetomarandaneto
committed
address pr review feedback
1 parent f50c80e commit b4e9886

4 files changed

Lines changed: 172 additions & 7 deletions

File tree

‎sdk_compliance_adapter/Program.cs‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System.Text.Json;
44
using System.Text.Json.Serialization;
55
using PostHog;
6+
using PostHog.Features;
67
using PostHog.Versioning;
78

89
var builder = WebApplication.CreateBuilder(args);
@@ -102,6 +103,36 @@
102103
return Results.Ok(new { success = true });
103104
});
104105

106+
app.MapPost("/get_feature_flag", async (GetFeatureFlagRequest request, CancellationToken cancellationToken) =>
107+
{
108+
if (state.Client is null)
109+
{
110+
return Results.BadRequest(new { error = "SDK not initialized" });
111+
}
112+
113+
if (string.IsNullOrEmpty(request.Key) || string.IsNullOrEmpty(request.DistinctId))
114+
{
115+
return Results.BadRequest(new { error = "key and distinct_id are required" });
116+
}
117+
118+
var options = new FeatureFlagOptions
119+
{
120+
PersonProperties = request.PersonProperties,
121+
Groups = BuildGroupCollection(request.Groups, request.GroupProperties),
122+
FlagKeysToEvaluate = [request.Key]
123+
};
124+
125+
#pragma warning disable CS0618 // Compliance adapter exercises the deprecated single-flag API contract.
126+
var flag = await state.Client.GetFeatureFlagAsync(
127+
request.Key,
128+
request.DistinctId,
129+
options,
130+
cancellationToken);
131+
#pragma warning restore CS0618
132+
133+
return Results.Ok(new { success = true, value = ToFeatureFlagValue(flag) });
134+
});
135+
105136
app.MapPost("/flush", async () =>
106137
{
107138
if (state.Client is null)
@@ -143,6 +174,30 @@
143174
return Results.Ok(new { success = true });
144175
});
145176

177+
static GroupCollection? BuildGroupCollection(
178+
Dictionary<string, string>? groups,
179+
Dictionary<string, Dictionary<string, object?>>? groupProperties)
180+
{
181+
if (groups is null or { Count: 0 })
182+
{
183+
return null;
184+
}
185+
186+
var collection = new GroupCollection();
187+
foreach (var (groupType, groupKey) in groups)
188+
{
189+
var properties = groupProperties?.GetValueOrDefault(groupType) ?? [];
190+
collection.Add(new Group(groupType, groupKey, properties));
191+
}
192+
193+
return collection;
194+
}
195+
196+
static object ToFeatureFlagValue(FeatureFlag? flag) =>
197+
flag is null
198+
? "undefined"
199+
: flag.VariantKey ?? (object)flag.IsEnabled;
200+
146201
app.Run();
147202

148203
// --- Models ---
@@ -170,6 +225,16 @@ record CaptureRequest(
170225
[property: JsonPropertyName("timestamp")] string? Timestamp = null
171226
);
172227

228+
record GetFeatureFlagRequest(
229+
[property: JsonPropertyName("key")] string Key,
230+
[property: JsonPropertyName("distinct_id")] string DistinctId,
231+
[property: JsonPropertyName("person_properties")] Dictionary<string, object?>? PersonProperties = null,
232+
[property: JsonPropertyName("groups")] Dictionary<string, string>? Groups = null,
233+
[property: JsonPropertyName("group_properties")] Dictionary<string, Dictionary<string, object?>>? GroupProperties = null,
234+
[property: JsonPropertyName("disable_geoip")] bool? DisableGeoip = null,
235+
[property: JsonPropertyName("force_remote")] bool? ForceRemote = null
236+
);
237+
173238
record StateResponse(
174239
[property: JsonPropertyName("pending_events")] int PendingEvents,
175240
[property: JsonPropertyName("total_events_captured")] int TotalEventsCaptured,

‎src/PostHog/Library/HttpClientExtensions.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ internal static class HttpClientExtensions
8686
continue;
8787
}
8888

89+
// Response processing is outside the try-catch so that exceptions from
90+
// EnsureSuccessfulApiCall (which may return HttpRequestException for 404s) won't
91+
// be caught by the retry logic above.
8992
using (response)
9093
{
9194
await response.EnsureSuccessfulApiCall(cancellationToken);

‎tests/UnitTests/Features/FeatureFlagsTests.cs‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3978,11 +3978,23 @@ public async Task DoesNotIncludeErrorPropertyWhenNoErrors()
39783978

39793979
[Fact]
39803980
public async Task IncludesTimeoutErrorWhenRequestTimesOut()
3981-
=> await AssertCapturedFeatureFlagErrorAsync(
3982-
handler => handler.AddFlagsResponseException(new TaskCanceledException("Request timed out")),
3983-
featureKey: "some-flag",
3984-
expectedResult: false,
3985-
expectedError: "timeout");
3981+
{
3982+
var container = new TestContainer(services => services.Configure<PostHogOptions>(options =>
3983+
{
3984+
options.FeatureFlagRequestMaxRetries = 0;
3985+
}));
3986+
container.FakeHttpMessageHandler.AddFlagsResponseException(new TaskCanceledException("Request timed out"));
3987+
var captureRequestHandler = container.FakeHttpMessageHandler.AddBatchResponse();
3988+
var client = container.Activate<PostHogClient>();
3989+
3990+
var result = await client.GetFeatureFlagAsync("some-flag", "distinct-id");
3991+
3992+
Assert.NotNull(result);
3993+
Assert.False(result.IsEnabled);
3994+
await client.FlushAsync();
3995+
var received = captureRequestHandler.GetReceivedRequestBody(indented: true);
3996+
Assert.Contains("\"$feature_flag_error\": \"timeout\"", received, StringComparison.Ordinal);
3997+
}
39863998

39873999
[Fact]
39884000
public async Task IncludesConnectionErrorWhenNetworkFails()

‎tests/UnitTests/Library/HttpClientExtensionsTests.cs‎

Lines changed: 87 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,7 @@ public class ThePostJsonWithNetworkRetryAsyncMethod
558558
{
559559
ProjectToken = "test-api-key",
560560
MaxRetries = maxRetries,
561+
FeatureFlagRequestMaxRetries = maxRetries,
561562
InitialRetryDelay = TimeSpan.FromMilliseconds(1),
562563
MaxRetryDelay = TimeSpan.FromSeconds(30)
563564
};
@@ -590,15 +591,74 @@ public async Task RetriesOnConnectionResetThenSucceeds()
590591
Assert.Equal(2, handler.RequestCount);
591592
}
592593

594+
[Fact]
595+
public async Task RetriesUntilSuccessAfterMultipleConnectionResetErrors()
596+
{
597+
var handler = new FakeRetryHttpMessageHandler();
598+
handler.AddException(new HttpRequestException("Connection reset", new SocketException((int)SocketError.ConnectionReset)));
599+
handler.AddException(new HttpRequestException("Connection reset", new SocketException((int)SocketError.ConnectionReset)));
600+
handler.AddException(new HttpRequestException("Connection reset", new SocketException((int)SocketError.ConnectionReset)));
601+
handler.AddResponse(HttpStatusCode.OK, new { flags = new { } });
602+
using var httpClient = CreateHttpClient(handler);
603+
var options = CreateOptions(maxRetries: 3);
604+
var timeProvider = new FakeTimeProvider();
605+
606+
var task = httpClient.PostJsonWithNetworkRetryAsync<FlagsApiResult>(
607+
FlagsUrl,
608+
new { api_key = "test", distinct_id = "user-1" },
609+
timeProvider,
610+
options,
611+
CancellationToken.None);
612+
613+
for (var i = 1; i <= 4 && !task.IsCompleted; i++)
614+
{
615+
await handler.WaitForRequestCountAsync(i);
616+
timeProvider.Advance(TimeSpan.FromMilliseconds(50));
617+
}
618+
619+
var result = await task;
620+
621+
Assert.NotNull(result);
622+
Assert.Equal(4, handler.RequestCount);
623+
}
624+
625+
[Fact]
626+
public async Task ThrowsAfterMaxRetriesWhenConnectionResetPersists()
627+
{
628+
var handler = new FakeRetryHttpMessageHandler();
629+
handler.AddException(new HttpRequestException("Connection reset", new SocketException((int)SocketError.ConnectionReset)));
630+
handler.AddException(new HttpRequestException("Connection reset", new SocketException((int)SocketError.ConnectionReset)));
631+
handler.AddException(new HttpRequestException("Connection reset", new SocketException((int)SocketError.ConnectionReset)));
632+
handler.AddException(new HttpRequestException("Connection reset", new SocketException((int)SocketError.ConnectionReset)));
633+
using var httpClient = CreateHttpClient(handler);
634+
var options = CreateOptions(maxRetries: 3);
635+
var timeProvider = new FakeTimeProvider();
636+
637+
var task = httpClient.PostJsonWithNetworkRetryAsync<FlagsApiResult>(
638+
FlagsUrl,
639+
new { api_key = "test", distinct_id = "user-1" },
640+
timeProvider,
641+
options,
642+
CancellationToken.None);
643+
644+
for (var i = 1; i <= 4 && !task.IsCompleted; i++)
645+
{
646+
await handler.WaitForRequestCountAsync(i);
647+
timeProvider.Advance(TimeSpan.FromMilliseconds(50));
648+
}
649+
650+
await Assert.ThrowsAsync<HttpRequestException>(() => task);
651+
Assert.Equal(4, handler.RequestCount);
652+
}
653+
593654
[Fact]
594655
public async Task DoesNotRetryWhenFeatureFlagRequestMaxRetriesIsZero()
595656
{
596657
var handler = new FakeRetryHttpMessageHandler();
597658
handler.AddException(new HttpRequestException("Connection reset", new SocketException((int)SocketError.ConnectionReset)));
598659
handler.AddResponse(HttpStatusCode.OK, new { flags = new { } });
599660
using var httpClient = CreateHttpClient(handler);
600-
var options = CreateOptions();
601-
options.FeatureFlagRequestMaxRetries = 0;
661+
var options = CreateOptions(maxRetries: 0);
602662
var timeProvider = new FakeTimeProvider();
603663

604664
await Assert.ThrowsAsync<HttpRequestException>(() =>
@@ -612,6 +672,31 @@ await Assert.ThrowsAsync<HttpRequestException>(() =>
612672
Assert.Equal(1, handler.RequestCount);
613673
}
614674

675+
#if NET8_0_OR_GREATER
676+
[Fact]
677+
public async Task DoesNotRetryOnUserCancellation()
678+
{
679+
var handler = new FakeRetryHttpMessageHandler();
680+
using var cts = new CancellationTokenSource();
681+
await cts.CancelAsync();
682+
handler.AddException(new TaskCanceledException("Operation was canceled.", null, cts.Token));
683+
handler.AddResponse(HttpStatusCode.OK, new { flags = new { } });
684+
using var httpClient = CreateHttpClient(handler);
685+
var options = CreateOptions();
686+
var timeProvider = new FakeTimeProvider();
687+
688+
await Assert.ThrowsAnyAsync<OperationCanceledException>(() =>
689+
httpClient.PostJsonWithNetworkRetryAsync<FlagsApiResult>(
690+
FlagsUrl,
691+
new { api_key = "test", distinct_id = "user-1" },
692+
timeProvider,
693+
options,
694+
cts.Token));
695+
696+
Assert.Equal(1, handler.RequestCount);
697+
}
698+
#endif
699+
615700
[Fact]
616701
public async Task DoesNotRetryConnectionRefused()
617702
{

0 commit comments

Comments
 (0)