Skip to content

chore: feature flag resiliency #38

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
EDsCODE merged 3 commits into from
Feb 14, 2023
Merged

chore: feature flag resiliency #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
85ad885
add timeout and error handling
EDsCODE Feb 7, 2023
ed38ad5
handle error
EDsCODE Feb 7, 2023
5737df3
add test
EDsCODE Feb 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion PostHog/Classes/PHGPayloadManager.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ - (void)reloadFeatureFlags
NSDictionary *flagPayloads = [responseDict objectForKey:@"featureFlagPayloads"];
[self receivedFeatureFlags:flags payloads:flagPayloads];
} failure:^(NSError * _Nonnull error) {
// TODO: handle error
return;
}];
}

Expand Down
16 changes: 13 additions & 3 deletions PostHog/Internal/PHGHTTPClient.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ - (instancetype)initWithRequestFactory:(PHGRequestFactory)requestFactory
self.requestFactory = requestFactory;
}
NSURLSessionConfiguration *config = [NSURLSessionConfiguration defaultSessionConfiguration];
config.timeoutIntervalForRequest = 10;

config.HTTPAdditionalHeaders = @{
@"Accept-Encoding" : @"gzip",
@"Content-Encoding" : @"gzip",
Expand Down Expand Up @@ -140,9 +142,17 @@ - (NSURLSessionDataTask *)sharedSessionUpload:(NSDictionary *)payload host:(NSUR
PHGLog(@"Error uploading request %@.", error);
failure(error);
} else {
NSDictionary *json = [NSJSONSerialization JSONObjectWithData:data options:0 error:nil];
NSLog(@"%@",json);
success(json);
NSInteger code = ((NSHTTPURLResponse *)response).statusCode;
if (code < 300) {

@neilkakkar neilkakkar Feb 7, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this be anything other than 200?

@EDsCODE EDsCODE Feb 13, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean based on what we send?

NSDictionary *json = [NSJSONSerialization JSONObjectWithData:data options:0 error:nil];
success(json);
}
NSError *error = [NSError errorWithDomain:NSURLErrorDomain
code:code
userInfo:nil];
PHGLog(@"Server responded with unexpected HTTP code.", error);
failure(error);

}
}];

Expand Down
19 changes: 19 additions & 0 deletions PostHogTests/FeatureFlagTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,25 @@ class FeatureFlagTests: QuickSpec {
let flagValue = posthog.getFeatureFlag("some-flag")
expect(flagValue).to(be("variant-1"))
}

it("bad request does not override current flags") {
_ = stubRequest("POST", "https://app.posthog.test/decide/?v=3" as LSMatcheable)
.andReturn(200)?
.withBody("{\"featureFlags\":{\"some-flag\":\"true\"}}" as LSHTTPBody);
posthog.reloadFeatureFlags()
// Hacky: Need to buffer for async request to happen without stub being cleaned up
sleep(1)
let isEnabled = posthog.isFeatureEnabled("some-flag")
expect(isEnabled).to(beTrue())

_ = stubRequest("POST", "https://app.posthog.test/decide/?v=3" as LSMatcheable)
.andReturn(400);
posthog.reloadFeatureFlags()
// Hacky: Need to buffer for async request to happen without stub being cleaned up
sleep(1)
let secondIsEnabled = posthog.isFeatureEnabled("some-flag")
expect(secondIsEnabled).to(beTrue())
}

}

Expand Down