Skip to content

Commit a278aab

Browse files
Add retry logic and increase timeout for CI end notification (#22)
Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 538c35f commit a278aab

File tree

6 files changed

+244
-36
lines changed

6 files changed

+244
-36
lines changed

README.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ For more information about JFrog Fly, see the [official documentation](https://d
1818
- ✅ OIDC authentication only
1919
- ✅ Allows ignoring specific package managers
2020
- ✅ Automatic CI session end notification to the Fly server
21+
- ✅ Retry mechanism with exponential backoff for CI notifications
2122

2223
## Usage
2324

@@ -51,8 +52,11 @@ This action requires OIDC authentication. The OIDC token is used to track upload
5152
permissions:
5253
contents: read
5354
id-token: write # Required for OIDC authentication
55+
actions: read # Optional: enables accurate job status detection
5456
```
5557
58+
> **Note**: The `actions: read` permission is optional but recommended. Without it, the action cannot accurately detect whether the job succeeded or failed, and will assume success. With this permission, the action can check the status of individual workflow steps to report accurate job status to the Fly server.
59+
5660
#### Required Inputs
5761

5862
- `url`: Fly URL

lib/index.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -585,21 +585,22 @@ function getErrorMessage(error) {
585585
exports.DEFAULT_HTTP_TIMEOUT_MS = 30000; // 30 seconds
586586
/**
587587
* Creates a configured HTTP client with sensible defaults.
588-
* Includes a 30-second timeout to prevent hanging requests.
588+
* Includes a configurable timeout to prevent hanging requests.
589589
*
590590
* @param userAgent - User agent string for the HTTP client (defaults to "fly-action")
591+
* @param timeoutMs - Socket timeout in milliseconds (defaults to DEFAULT_HTTP_TIMEOUT_MS)
591592
* @returns Configured HttpClient instance
592593
*
593594
* @example
594595
* const client = createHttpClient();
595596
* const response = await client.get("https://api.example.com");
596597
*
597598
* @example
598-
* const client = createHttpClient("my-custom-agent");
599+
* const client = createHttpClient("my-custom-agent", 60000);
599600
*/
600-
function createHttpClient(userAgent = "fly-action") {
601+
function createHttpClient(userAgent = "fly-action", timeoutMs = exports.DEFAULT_HTTP_TIMEOUT_MS) {
601602
return new http_client_1.HttpClient(userAgent, undefined, {
602-
socketTimeout: exports.DEFAULT_HTTP_TIMEOUT_MS,
603+
socketTimeout: timeoutMs,
603604
});
604605
}
605606

lib/post.js

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,43 @@ const github = __importStar(__nccwpck_require__(3228));
152152
const constants_1 = __nccwpck_require__(9920);
153153
const utils_1 = __nccwpck_require__(9236);
154154
const job_summary_1 = __nccwpck_require__(5175);
155+
// Retry configuration
156+
const MAX_RETRIES = 3;
157+
const INITIAL_RETRY_DELAY_MS = 1000; // 1 second
158+
const REQUEST_TIMEOUT_MS = 60000; // 60 seconds (Wingman AI calls can take 30+ seconds)
159+
/**
160+
* Sleep for a given number of milliseconds
161+
*/
162+
function sleep(ms) {
163+
return new Promise((resolve) => setTimeout(resolve, ms));
164+
}
165+
/**
166+
* Execute an HTTP POST request with retry logic and exponential backoff
167+
*/
168+
async function postWithRetry(httpClient, url, body, headers) {
169+
let lastError;
170+
for (let attempt = 1; attempt <= MAX_RETRIES; attempt++) {
171+
try {
172+
core.info(`[${new Date().toISOString()}] Attempt ${attempt}/${MAX_RETRIES} - Sending request to ${url}`);
173+
const response = await httpClient.post(url, body, headers);
174+
// If we get a response (even an error response), return it
175+
// The caller will handle non-200 status codes
176+
return response;
177+
}
178+
catch (error) {
179+
lastError = error instanceof Error ? error : new Error(String(error));
180+
if (attempt < MAX_RETRIES) {
181+
const delayMs = INITIAL_RETRY_DELAY_MS * Math.pow(2, attempt - 1);
182+
core.warning(`Request failed (attempt ${attempt}/${MAX_RETRIES}): ${lastError.message}. Retrying in ${delayMs}ms...`);
183+
await sleep(delayMs);
184+
}
185+
else {
186+
core.error(`Request failed after ${MAX_RETRIES} attempts: ${lastError.message}`);
187+
}
188+
}
189+
}
190+
throw lastError || new Error("Request failed after retries");
191+
}
155192
/**
156193
* Gets GitHub environment variables required for job status checking
157194
*/
@@ -248,8 +285,18 @@ async function determineJobStatus() {
248285
return constants_1.GITHUB_STATUS_SUCCESS;
249286
}
250287
catch (apiError) {
251-
core.warning(`Failed to check job status via GitHub API: ${apiError}`);
252-
core.warning("Falling back to assuming job succeeded since post action is running");
288+
// Check if this is a permission error (expected when actions:read is not granted)
289+
const errorMessage = String(apiError);
290+
const isPermissionError = errorMessage.includes("Resource not accessible by integration") ||
291+
errorMessage.includes("403");
292+
if (isPermissionError) {
293+
core.info("ℹ️ Cannot access workflow job status (requires 'actions: read' permission). " +
294+
"Assuming job succeeded since post action is running.");
295+
}
296+
else {
297+
core.warning(`Failed to check job status via GitHub API: ${apiError}`);
298+
core.info("Assuming job succeeded since post action is running.");
299+
}
253300
return constants_1.GITHUB_STATUS_SUCCESS;
254301
}
255302
}
@@ -292,10 +339,10 @@ async function runPost() {
292339
}
293340
core.info(`Fly API URL: ${flyUrl}/fly/api/v1/ci/end`); // Changed from debug to info
294341
core.info(`Request payload: ${JSON.stringify(payload)}`);
295-
const httpClient = (0, utils_1.createHttpClient)();
342+
const httpClient = (0, utils_1.createHttpClient)("fly-action", REQUEST_TIMEOUT_MS);
296343
core.info(`[${new Date().toISOString()}] Attempting to send CI end notification to Fly...`);
297344
try {
298-
const response = await httpClient.post(`${flyUrl}/fly/api/v1/ci/end`, JSON.stringify(payload), {
345+
const response = await postWithRetry(httpClient, `${flyUrl}/fly/api/v1/ci/end`, JSON.stringify(payload), {
299346
Authorization: `Bearer ${accessToken}`,
300347
"content-type": "application/json",
301348
});
@@ -433,21 +480,22 @@ function getErrorMessage(error) {
433480
exports.DEFAULT_HTTP_TIMEOUT_MS = 30000; // 30 seconds
434481
/**
435482
* Creates a configured HTTP client with sensible defaults.
436-
* Includes a 30-second timeout to prevent hanging requests.
483+
* Includes a configurable timeout to prevent hanging requests.
437484
*
438485
* @param userAgent - User agent string for the HTTP client (defaults to "fly-action")
486+
* @param timeoutMs - Socket timeout in milliseconds (defaults to DEFAULT_HTTP_TIMEOUT_MS)
439487
* @returns Configured HttpClient instance
440488
*
441489
* @example
442490
* const client = createHttpClient();
443491
* const response = await client.get("https://api.example.com");
444492
*
445493
* @example
446-
* const client = createHttpClient("my-custom-agent");
494+
* const client = createHttpClient("my-custom-agent", 60000);
447495
*/
448-
function createHttpClient(userAgent = "fly-action") {
496+
function createHttpClient(userAgent = "fly-action", timeoutMs = exports.DEFAULT_HTTP_TIMEOUT_MS) {
449497
return new http_client_1.HttpClient(userAgent, undefined, {
450-
socketTimeout: exports.DEFAULT_HTTP_TIMEOUT_MS,
498+
socketTimeout: timeoutMs,
451499
});
452500
}
453501

src/post.spec.ts

Lines changed: 98 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ describe("runPost", () => {
236236
);
237237
});
238238

239-
it("should re-throw errors during HTTP client post operation", async () => {
239+
it("should re-throw errors after all retry attempts fail", async () => {
240240
// Mock successful workflow for status checking
241241
const workflowRun = createMockWorkflowRun("in_progress", null);
242242
const jobs = {
@@ -258,10 +258,56 @@ describe("runPost", () => {
258258
if (name === STATE_FLY_ACCESS_TOKEN) return "test-access-token";
259259
return "";
260260
});
261+
// Mock all retries failing
261262
mockHttpClientPost.mockRejectedValue(new Error("Network error"));
262263

263264
await expect(runPost()).rejects.toThrow("Network error");
264-
});
265+
// Should have tried 3 times (MAX_RETRIES)
266+
expect(mockHttpClientPost).toHaveBeenCalledTimes(3);
267+
}, 15000); // Increase timeout for retry delays
268+
269+
it("should succeed on retry after initial failure", async () => {
270+
// Mock successful workflow for status checking
271+
const workflowRun = createMockWorkflowRun("in_progress", null);
272+
const jobs = {
273+
jobs: [
274+
createMockJob("test-job", "in_progress", null, [
275+
createMockStep("Checkout", "success"),
276+
]),
277+
],
278+
};
279+
280+
const mockOctokit = createMockOctokit(workflowRun, jobs);
281+
mockGithub.getOctokit.mockReturnValue(
282+
mockOctokit as unknown as ReturnType<typeof mockGithub.getOctokit>,
283+
);
284+
285+
mockCore.getState.mockImplementation((name: string) => {
286+
if (name === STATE_FLY_URL) return "https://fly.example.com";
287+
if (name === STATE_FLY_ACCESS_TOKEN) return "test-access-token";
288+
if (name === STATE_FLY_PACKAGE_MANAGERS) return JSON.stringify(["npm"]);
289+
return "";
290+
});
291+
292+
// First call fails, second succeeds
293+
const fakeResponse: HttpClientResponse = {
294+
message: { statusCode: 200, headers: {} as IncomingHttpHeaders },
295+
readBody: async () => "Notification sent",
296+
} as unknown as HttpClientResponse;
297+
mockHttpClientPost
298+
.mockRejectedValueOnce(new Error("Timeout"))
299+
.mockResolvedValueOnce(fakeResponse);
300+
301+
await runPost();
302+
303+
expect(mockHttpClientPost).toHaveBeenCalledTimes(2);
304+
expect(mockCore.warning).toHaveBeenCalledWith(
305+
expect.stringContaining("Request failed (attempt 1/3)"),
306+
);
307+
expect(mockCore.info).toHaveBeenCalledWith(
308+
"✅ CI end notification completed successfully",
309+
);
310+
}, 15000);
265311

266312
it("should re-throw error if HTTP response is not 200", async () => {
267313
// Mock successful workflow for status checking
@@ -389,11 +435,45 @@ describe("runPost", () => {
389435
expect(mockCore.warning).toHaveBeenCalledWith(
390436
"Failed to check job status via GitHub API: Error: API Error",
391437
);
392-
expect(mockHttpClientPost).toHaveBeenCalledWith(
393-
expect.any(String),
394-
JSON.stringify({ status: "success", package_managers: ["npm", "maven"] }),
395-
expect.any(Object),
438+
expect(mockHttpClientPost).toHaveBeenCalled();
439+
});
440+
441+
it("should handle GitHub API permission error gracefully without warning", async () => {
442+
// Mock GitHub API permission error (expected when actions:read is not granted)
443+
const mockOctokit = {
444+
rest: {
445+
actions: {
446+
listJobsForWorkflowRun: jest
447+
.fn()
448+
.mockRejectedValue(
449+
new Error(
450+
"Resource not accessible by integration - https://docs.github.com/rest/actions/workflow-jobs",
451+
),
452+
),
453+
},
454+
},
455+
};
456+
mockGithub.getOctokit.mockReturnValue(
457+
mockOctokit as unknown as ReturnType<typeof mockGithub.getOctokit>,
396458
);
459+
460+
const fakeResponse: HttpClientResponse = {
461+
message: { statusCode: 200, headers: {} as IncomingHttpHeaders },
462+
readBody: async () => "Notification sent",
463+
} as unknown as HttpClientResponse;
464+
mockHttpClientPost.mockResolvedValue(fakeResponse);
465+
466+
await runPost();
467+
468+
// Should use info instead of warning for permission errors
469+
expect(mockCore.info).toHaveBeenCalledWith(
470+
expect.stringContaining("Cannot access workflow job status"),
471+
);
472+
// Should NOT emit warning for permission errors
473+
expect(mockCore.warning).not.toHaveBeenCalledWith(
474+
expect.stringContaining("Failed to check job status via GitHub API"),
475+
);
476+
expect(mockHttpClientPost).toHaveBeenCalled();
397477
});
398478

399479
it("should exclude post action steps from failure detection", async () => {
@@ -793,7 +873,7 @@ describe("runPostScriptLogic", () => {
793873
expect(core.setFailed).not.toHaveBeenCalled();
794874
});
795875

796-
it("should call runPost and setFailed on error", async () => {
876+
it("should call runPost and setFailed on error after all retries", async () => {
797877
// Mock successful workflow setup but HTTP error
798878
const workflowRun = createMockWorkflowRun("in_progress", null);
799879
const jobs = {
@@ -810,15 +890,17 @@ describe("runPostScriptLogic", () => {
810890
);
811891

812892
const errorMessage = "Test error from runPost";
813-
mockHttpClientPost.mockRejectedValueOnce(new Error(errorMessage));
893+
// Reject all retry attempts
894+
mockHttpClientPost.mockRejectedValue(new Error(errorMessage));
814895

815896
await runPostScriptLogic();
816897

817-
expect(mockHttpClientPost).toHaveBeenCalledTimes(1);
898+
// Should have tried 3 times (MAX_RETRIES)
899+
expect(mockHttpClientPost).toHaveBeenCalledTimes(3);
818900
expect(core.setFailed).toHaveBeenCalledWith(errorMessage);
819-
});
901+
}, 15000);
820902

821-
it("should handle non-Error objects thrown by runPost", async () => {
903+
it("should handle non-Error objects thrown by runPost after all retries", async () => {
822904
// Mock successful workflow setup but string error
823905
const workflowRun = createMockWorkflowRun("in_progress", null);
824906
const jobs = {
@@ -835,11 +917,13 @@ describe("runPostScriptLogic", () => {
835917
);
836918

837919
const errorString = "Just a string error";
838-
mockHttpClientPost.mockRejectedValueOnce(errorString);
920+
// Reject all retry attempts
921+
mockHttpClientPost.mockRejectedValue(errorString);
839922

840923
await runPostScriptLogic();
841924

842-
expect(mockHttpClientPost).toHaveBeenCalledTimes(1);
925+
// Should have tried 3 times (MAX_RETRIES)
926+
expect(mockHttpClientPost).toHaveBeenCalledTimes(3);
843927
expect(core.setFailed).toHaveBeenCalledWith(errorString);
844-
});
928+
}, 15000);
845929
});

0 commit comments

Comments
 (0)