-
Notifications
You must be signed in to change notification settings - Fork 10
feat: implement cancelAllPipelines for CI providers #205
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -168,6 +168,41 @@ export class AzurePipelineService { | |
| } | ||
| } | ||
|
|
||
| public async cancelBuild(buildId: number): Promise<void> { | ||
| try { | ||
| await retry( | ||
| async () => { | ||
| await this.client.patch( | ||
| `${this.project}/_apis/build/builds/${buildId}?${this.getApiVersionParam()}`, | ||
| { status: 'cancelling' } | ||
| ); | ||
| }, | ||
| { | ||
| retries: 3, | ||
| minTimeout: 1000, | ||
| maxTimeout: 5000, | ||
| onRetry: (error: Error, attempt: number) => { | ||
| console.log(`[Azure] Retry ${attempt}/3 - Cancelling build ${buildId}: ${error.message}`); | ||
| }, | ||
| } | ||
| ); | ||
|
|
||
| console.log(`[Azure] Successfully cancelled build ${buildId}`); | ||
| } catch (error: any) { | ||
| // Handle specific error cases | ||
| if (error.response?.status === 404) { | ||
| throw new Error(`Build ${buildId} not found`); | ||
| } | ||
| if (error.response?.status === 403) { | ||
| throw new Error(`Insufficient permissions to cancel build ${buildId}`); | ||
| } | ||
| if (error.response?.status === 400) { | ||
| throw new Error(`Build ${buildId} cannot be cancelled (already completed or not cancellable)`); | ||
| } | ||
| throw new Error(`Failed to cancel build ${buildId}: ${error.message}`); | ||
|
Comment on lines
+191
to
+202
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use safer error type checking and avoid The catch block uses Apply this diff to use proper type guards: - } catch (error: any) {
+ } catch (error: unknown) {
// Handle specific error cases
- if (error.response?.status === 404) {
+ if (axios.isAxiosError(error) && error.response?.status === 404) {
throw new Error(`Build ${buildId} not found`);
}
- if (error.response?.status === 403) {
+ if (axios.isAxiosError(error) && error.response?.status === 403) {
throw new Error(`Insufficient permissions to cancel build ${buildId}`);
}
- if (error.response?.status === 400) {
+ if (axios.isAxiosError(error) && error.response?.status === 400) {
throw new Error(`Build ${buildId} cannot be cancelled (already completed or not cancellable)`);
}
- throw new Error(`Failed to cancel build ${buildId}: ${error.message}`);
+ const message = error instanceof Error ? error.message : String(error);
+ throw new Error(`Failed to cancel build ${buildId}: ${message}`);
}🤖 Prompt for AI Agents |
||
| } | ||
| } | ||
|
|
||
| public async getAllPipelines(): Promise<AzurePipelineDefinition[]> { | ||
| try { | ||
| const pipelines = await this.client.get<{ value: AzurePipelineDefinition[] }>( | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -590,4 +590,74 @@ export class GithubActionsService { | |||||
| throw new GithubApiError(`Error finding workflow run for commit ${sha.substring(0, 7)}`, error.status, error); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Cancel a workflow run | ||||||
| * | ||||||
| * @param owner Repository owner | ||||||
| * @param repo Repository name | ||||||
| * @param runId Workflow run ID to cancel | ||||||
| * @returns Promise resolving when the workflow run is cancelled | ||||||
| * @throws GithubNotFoundError if the workflow run is not found | ||||||
| * @throws GithubApiError for other API errors | ||||||
| */ | ||||||
| async cancelWorkflowRun( | ||||||
| owner: string, | ||||||
| repo: string, | ||||||
| runId: number | ||||||
| ): Promise<void> { | ||||||
| try { | ||||||
| await retry( | ||||||
| async () => { | ||||||
| await this.octokit.actions.cancelWorkflowRun({ | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix the API path to include All other methods in this service use Apply this diff: - await this.octokit.actions.cancelWorkflowRun({
+ await this.octokit.rest.actions.cancelWorkflowRun({📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| owner, | ||||||
| repo, | ||||||
| run_id: runId, | ||||||
| }); | ||||||
| }, | ||||||
| { | ||||||
| retries: this.maxRetries, | ||||||
| minTimeout: this.minTimeout, | ||||||
| maxTimeout: this.maxTimeout, | ||||||
| factor: this.factor, | ||||||
| onRetry: (error: Error, attempt: number) => { | ||||||
| console.log( | ||||||
| `[GitHub Actions] Retry ${attempt}/${this.maxRetries} - Cancelling workflow run ${runId}: ${error.message}` | ||||||
| ); | ||||||
| }, | ||||||
| } | ||||||
| ); | ||||||
|
|
||||||
| console.log(`[GitHub Actions] Successfully cancelled workflow run ${runId}`); | ||||||
| } catch (error: any) { | ||||||
| // Handle specific error cases | ||||||
| if (error.status === 404) { | ||||||
| throw new GithubNotFoundError( | ||||||
| 'workflow run', | ||||||
| `${runId} in repository ${owner}/${repo}` | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
| if (error.status === 403) { | ||||||
| throw new GithubApiError( | ||||||
| `Insufficient permissions to cancel workflow run ${runId}`, | ||||||
| error.status | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
| if (error.status === 409) { | ||||||
| throw new GithubApiError( | ||||||
| `Workflow run ${runId} cannot be cancelled (already completed or not cancellable)`, | ||||||
| error.status | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
| // Re-throw with more context | ||||||
| throw new GithubApiError( | ||||||
| `Failed to cancel workflow run ${runId}: ${error.message}`, | ||||||
| error.status || 500, | ||||||
| error | ||||||
| ); | ||||||
| } | ||||||
|
Comment on lines
+610
to
+661
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use The current implementation retries all errors, including 404 (not found), 403 (permissions), and 409 (already completed). These errors are non-retryable and should use the Apply this diff: try {
await retry(
- async () => {
+ async (bail) => {
await this.octokit.rest.actions.cancelWorkflowRun({
owner,
repo,
run_id: runId,
});
+ },
+ {
+ retries: this.maxRetries,
+ minTimeout: this.minTimeout,
+ maxTimeout: this.maxTimeout,
+ factor: this.factor,
+ onRetry: (error: Error, attempt: number) => {
+ console.log(
+ `[GitHub Actions] Retry ${attempt}/${this.maxRetries} - Cancelling workflow run ${runId}: ${error.message}`
+ );
+ },
+ }
+ );
+
+ console.log(`[GitHub Actions] Successfully cancelled workflow run ${runId}`);
+ } catch (error: any) {
+ // Handle specific error cases - bail immediately for non-retryable errors
+ if (error.status === 404) {
+ throw new GithubNotFoundError(
+ 'workflow run',
+ `${runId} in repository ${owner}/${repo}`
+ );
+ }
+
+ if (error.status === 403) {
+ throw new GithubApiError(
+ `Insufficient permissions to cancel workflow run ${runId}`,
+ error.status
+ );
+ }
+
+ if (error.status === 409) {
+ throw new GithubApiError(
+ `Workflow run ${runId} cannot be cancelled (already completed or not cancellable)`,
+ error.status
+ );
+ }
+
+ // Re-throw with more context
+ throw new GithubApiError(
+ `Failed to cancel workflow run ${runId}: ${error.message}`,
+ error.status || 500,
+ error
+ );
+ }should be: try {
await retry(
async (bail) => {
- await this.octokit.rest.actions.cancelWorkflowRun({
- owner,
- repo,
- run_id: runId,
- });
+ try {
+ await this.octokit.rest.actions.cancelWorkflowRun({
+ owner,
+ repo,
+ run_id: runId,
+ });
+ } catch (error: any) {
+ // Bail immediately for non-retryable errors
+ if (error.status === 404) {
+ bail(new GithubNotFoundError(
+ 'workflow run',
+ `${runId} in repository ${owner}/${repo}`,
+ error.status
+ ));
+ return;
+ }
+
+ if (error.status === 403) {
+ bail(new GithubApiError(
+ `Insufficient permissions to cancel workflow run ${runId}`,
+ error.status
+ ));
+ return;
+ }
+
+ if (error.status === 409) {
+ bail(new GithubApiError(
+ `Workflow run ${runId} cannot be cancelled (already completed or not cancellable)`,
+ error.status
+ ));
+ return;
+ }
+
+ // Throw for retryable errors
+ throw error;
+ }
},
{
retries: this.maxRetries,
minTimeout: this.minTimeout,
maxTimeout: this.maxTimeout,
factor: this.factor,
onRetry: (error: Error, attempt: number) => {
console.log(
`[GitHub Actions] Retry ${attempt}/${this.maxRetries} - Cancelling workflow run ${runId}: ${error.message}`
);
},
}
);
console.log(`[GitHub Actions] Successfully cancelled workflow run ${runId}`);
} catch (error: any) {
- // Handle specific error cases
- if (error.status === 404) {
- throw new GithubNotFoundError(
- 'workflow run',
- `${runId} in repository ${owner}/${repo}`
- );
- }
-
- if (error.status === 403) {
- throw new GithubApiError(
- `Insufficient permissions to cancel workflow run ${runId}`,
- error.status
- );
- }
-
- if (error.status === 409) {
- throw new GithubApiError(
- `Workflow run ${runId} cannot be cancelled (already completed or not cancellable)`,
- error.status
- );
- }
-
- // Re-throw with more context
+ // Add context to unknown errors
throw new GithubApiError(
`Failed to cancel workflow run ${runId}: ${error.message}`,
error.status || 500,
error
);
}
🤖 Prompt for AI Agents |
||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -165,6 +165,25 @@ export class JenkinsBuildService { | |||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||
| * Stop/abort a running build | ||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||
| async stopBuild(jobName: string, buildNumber: number, folderName?: string): Promise<void> { | ||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||
| const path = JenkinsPathBuilder.buildBuildPath( | ||||||||||||||||||||||||||||||||
| jobName, | ||||||||||||||||||||||||||||||||
| buildNumber, | ||||||||||||||||||||||||||||||||
| folderName, | ||||||||||||||||||||||||||||||||
| 'stop' | ||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| await this.httpClient.post(path, null); | ||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||
| // If build is not found or already stopped, throw specific error | ||||||||||||||||||||||||||||||||
| throw new JenkinsBuildNotFoundError(jobName, buildNumber, folderName); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
Comment on lines
+181
to
+184
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Preserve non-404 failures from stopBuild Catching every failure and rethrowing - } catch (error) {
- // If build is not found or already stopped, throw specific error
- throw new JenkinsBuildNotFoundError(jobName, buildNumber, folderName);
- }
+ } catch (error) {
+ const status =
+ (error as { statusCode?: number })?.statusCode ??
+ (error as { response?: { status?: number } })?.response?.status;
+
+ if (status === 404) {
+ throw new JenkinsBuildNotFoundError(jobName, buildNumber, folderName);
+ }
+
+ throw error;
+ }📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||
| * Wait for a build to complete with timeout | ||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -265,6 +265,37 @@ export class KubeClient { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Generic method to patch a resource with proper error handling | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @template T The resource type to return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {K8sApiOptions} options - API options for the request (must include name) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param {any} patchData - The patch data to apply | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @returns {Promise<T>} The patched resource of type T | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public async patchResource<T>(options: K8sApiOptions, patchData: any): Promise<T> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!options.name) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error('Resource name is required for patchResource'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const response = await this.customApi.patchNamespacedCustomObject({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| group: options.group, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| version: options.version, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| namespace: options.namespace, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| plural: options.plural, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name: options.name, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| body: patchData, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return response as T; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.error( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `Error patching resource '${options.name}' in namespace '${options.namespace}': ${error}` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error(`Failed to patch resource '${options.name}': ${error}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+276
to
+296
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Set merge-patch Content-Type when patching
const response = await this.customApi.patchNamespacedCustomObject({
group: options.group,
version: options.version,
namespace: options.namespace,
plural: options.plural,
name: options.name,
body: patchData,
+ headers: {
+ 'Content-Type': 'application/merge-patch+json',
+ },
});📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Retrieves logs from a pod or specific containers within a pod | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param podName The name of the pod | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-retryable errors should not trigger retries.
The retry mechanism will attempt 3 retries for all errors, including HTTP 400, 403, and 404 responses. These status codes indicate permanent failures (bad request, forbidden, not found) that won't succeed on retry, wasting time and resources.
Refactor to check the error status and bail early for non-retryable errors:
await retry( async () => { - await this.client.patch( + try { + await this.client.patch( `${this.project}/_apis/build/builds/${buildId}?${this.getApiVersionParam()}`, { status: 'cancelling' } ); + } catch (error: any) { + // Don't retry on permanent failures + if (error.response?.status && [400, 403, 404].includes(error.response.status)) { + throw new retry.AbortError(error); + } + throw error; + } }, { retries: 3,