Skip to content

Commit ee5ede7

Browse files
committed
Address review comments
1 parent edfcb0a commit ee5ede7

File tree

3 files changed

+27
-5
lines changed

3 files changed

+27
-5
lines changed

lib/analyze-action.js

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

src/database-upload.test.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ test.serial(
225225
.returns("true");
226226
sinon.stub(gitUtils, "isAnalyzingDefaultBranch").resolves(true);
227227

228-
await mockHttpRequests(422);
228+
const databaseUploadSpy = await mockHttpRequests(422);
229229

230230
const loggedMessages = [] as LoggedMessage[];
231231
await cleanupAndUploadDatabases(
@@ -245,6 +245,9 @@ test.serial(
245245
"Failed to upload database for javascript: some error message",
246246
) !== undefined,
247247
);
248+
249+
// Non-retryable errors should not be retried.
250+
t.is(databaseUploadSpy.callCount, 1);
248251
});
249252
},
250253
);
@@ -260,11 +263,11 @@ test.serial(
260263
.returns("true");
261264
sinon.stub(gitUtils, "isAnalyzingDefaultBranch").resolves(true);
262265

263-
await mockHttpRequests(500);
266+
const databaseUploadSpy = await mockHttpRequests(500);
264267

265268
// Stub setTimeout to fire immediately to avoid real delays from retry backoff.
266269
const originalSetTimeout = global.setTimeout;
267-
sinon
270+
const setTimeoutStub = sinon
268271
.stub(global, "setTimeout")
269272
.callsFake((fn: () => void) => originalSetTimeout(fn, 0));
270273

@@ -286,6 +289,15 @@ test.serial(
286289
"Failed to upload database for javascript: some error message",
287290
) !== undefined,
288291
);
292+
293+
// Retryable errors should be retried the expected number of times.
294+
t.is(databaseUploadSpy.callCount, 4);
295+
296+
// setTimeout should have been called with the expected backoff delays.
297+
const setTimeoutDelays = setTimeoutStub.args.map(
298+
(args) => args[1] as number,
299+
);
300+
t.deepEqual(setTimeoutDelays, [15_000, 30_000, 60_000]);
289301
});
290302
},
291303
);

src/database-upload.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,12 @@ export async function cleanupAndUploadDatabases(
147147
const httpError = asHTTPError(e);
148148
const isRetryable =
149149
!httpError || !DO_NOT_RETRY_STATUSES.includes(httpError.status);
150-
if (!isRetryable || attempt === maxAttempts) {
150+
if (!isRetryable) {
151+
throw e;
152+
} else if (attempt === maxAttempts) {
153+
logger.error(
154+
`Maximum retry attempts exhausted (${attempt}), aborting database upload`,
155+
);
151156
throw e;
152157
}
153158
const backoffMs = 15_000 * Math.pow(2, attempt - 1); // 15s, 30s, 60s

0 commit comments

Comments
 (0)