Skip to content

Commit 621e079

Browse files
committed
Throw an error if the feature flag API request errors
1 parent d6499fa commit 621e079

6 files changed

Lines changed: 28 additions & 37 deletions

File tree

lib/feature-flags.js

Lines changed: 5 additions & 3 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: 7 additions & 11 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: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import * as sinon from "sinon";
55
import * as apiClient from "./api-client";
66
import { GitHubApiDetails } from "./api-client";
77
import { GitHubFeatureFlags } from "./feature-flags";
8+
import { getRunnerLogger } from "./logging";
89
import {
910
getRecordingLogger,
1011
LoggedMessage,
@@ -135,31 +136,22 @@ test("Feature flags are disabled if they're not returned in API response", async
135136
});
136137
});
137138

138-
test("All feature flags are disabled if the API request errors", async (t) => {
139+
test("Feature flags exception is propagated if the API request errors", async (t) => {
139140
await withTmpDir(async (tmpDir) => {
140141
setupActionsVars(tmpDir, tmpDir);
141142

142-
const loggedMessages = [];
143143
const featureFlags = new GitHubFeatureFlags(
144144
{ type: GitHubVariant.DOTCOM },
145145
testApiDetails,
146-
getRecordingLogger(loggedMessages)
146+
getRunnerLogger(true)
147147
);
148148

149149
mockHttpRequests(500, {});
150150

151-
t.assert((await featureFlags.getDatabaseUploadsEnabled()) === false);
152-
t.assert((await featureFlags.getMlPoweredQueriesEnabled()) === false);
153-
t.assert((await featureFlags.getUploadsDomainEnabled()) === false);
154-
155-
t.assert(
156-
loggedMessages.find(
157-
(v: LoggedMessage) =>
158-
v.type === "info" &&
159-
v.message ===
160-
"Disabling all feature flags due to unknown error: Error: some error message"
161-
) !== undefined
162-
);
151+
await t.throwsAsync(async () => featureFlags.preloadFeatureFlags(), {
152+
message:
153+
"Encountered an error while trying to load feature flags: Error: some error message",
154+
});
163155
});
164156
});
165157

@@ -174,11 +166,10 @@ for (const featureFlag of FEATURE_FLAGS) {
174166
await withTmpDir(async (tmpDir) => {
175167
setupActionsVars(tmpDir, tmpDir);
176168

177-
const loggedMessages = [];
178169
const featureFlags = new GitHubFeatureFlags(
179170
{ type: GitHubVariant.DOTCOM },
180171
testApiDetails,
181-
getRecordingLogger(loggedMessages)
172+
getRunnerLogger(true)
182173
);
183174

184175
const expectedFeatureFlags = {};

src/feature-flags.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,13 @@ export class GitHubFeatureFlags implements FeatureFlags {
7575
);
7676
return response.data;
7777
} catch (e) {
78-
console.log(e);
79-
this.logger.info(
80-
`Disabling all feature flags due to unknown error: ${e}`
78+
// Some feature flags, such as `ml_powered_queries_enabled` affect the produced alerts.
79+
// Considering these feature flags disabled in the event of a transient error could
80+
// therefore lead to alert churn. As a result, we crash if we cannot determine the value of
81+
// the feature flags.
82+
throw new Error(
83+
`Encountered an error while trying to load feature flags: ${e}`
8184
);
82-
return {};
8385
}
8486
};
8587

0 commit comments

Comments
 (0)