Skip to content

Commit ba95eeb

Browse files
authored
Merge pull request #1145 from github/criemen/fix-ff-crash
Don't crash if we are unable to get a response from the feature-flag endpoint.
2 parents d8c9c72 + c059f95 commit ba95eeb

6 files changed

Lines changed: 65 additions & 6 deletions

File tree

lib/feature-flags.js

Lines changed: 7 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/feature-flags.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/feature-flags.test.js

Lines changed: 16 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/feature-flags.test.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/feature-flags.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,37 @@ for (const variant of ALL_FEATURE_FLAGS_DISABLED_VARIANTS) {
6767
});
6868
}
6969

70+
test("API response missing", async (t) => {
71+
await withTmpDir(async (tmpDir) => {
72+
setupActionsVars(tmpDir, tmpDir);
73+
74+
const loggedMessages = [];
75+
const featureFlags = new GitHubFeatureFlags(
76+
{ type: GitHubVariant.DOTCOM },
77+
testApiDetails,
78+
testRepositoryNwo,
79+
getRecordingLogger(loggedMessages)
80+
);
81+
82+
mockFeatureFlagApiEndpoint(403, {});
83+
84+
for (const flag of Object.values(FeatureFlag)) {
85+
t.assert((await featureFlags.getValue(flag)) === false);
86+
}
87+
88+
for (const featureFlag of ["ml_powered_queries_enabled"]) {
89+
t.assert(
90+
loggedMessages.find(
91+
(v: LoggedMessage) =>
92+
v.type === "debug" &&
93+
v.message ===
94+
`No feature flags API response for ${featureFlag}, considering it disabled.`
95+
) !== undefined
96+
);
97+
}
98+
});
99+
});
100+
70101
test("Feature flags are disabled if they're not returned in API response", async (t) => {
71102
await withTmpDir(async (tmpDir) => {
72103
setupActionsVars(tmpDir, tmpDir);

src/feature-flags.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,21 @@ export class GitHubFeatureFlags implements FeatureFlags {
3131
) {}
3232

3333
async getValue(flag: FeatureFlag): Promise<boolean> {
34-
const response = (await this.getApiResponse())[flag];
34+
const response = await this.getApiResponse();
3535
if (response === undefined) {
36+
this.logger.debug(
37+
`No feature flags API response for ${flag}, considering it disabled.`
38+
);
39+
return false;
40+
}
41+
const flagValue = response[flag];
42+
if (flagValue === undefined) {
3643
this.logger.debug(
3744
`Feature flag '${flag}' undefined in API response, considering it disabled.`
3845
);
3946
return false;
4047
}
41-
return response;
48+
return flagValue;
4249
}
4350

4451
private async getApiResponse(): Promise<FeatureFlagsApiResponse> {

0 commit comments

Comments
 (0)