Skip to content

Commit

Permalink
src: alert if an exception is thrown on cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
adityamaru committed Dec 9, 2024
1 parent 1cc1561 commit 97dd12c
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 12 deletions.
2 changes: 1 addition & 1 deletion dist/index.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/index.js.map

Large diffs are not rendered by default.

14 changes: 7 additions & 7 deletions src/__tests__/blacksmith-builder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('startBlacksmithBuilder', () => {

expect(result).toEqual({addr: null, buildId: null, exposeId: ''});
expect(core.warning).toHaveBeenCalledWith('Error during Blacksmith builder setup: Failed to resolve dockerfile path. Falling back to a local build.');
expect(reporter.reportBuilderCreationFailed).toHaveBeenCalledWith(new Error('Failed to resolve dockerfile path'));
expect(reporter.reportBuildPushActionFailure).toHaveBeenCalledWith(new Error('Failed to resolve dockerfile path'));
});

test('should handle missing dockerfile path with nofallback=true', async () => {
Expand All @@ -55,7 +55,7 @@ describe('startBlacksmithBuilder', () => {

await expect(main.startBlacksmithBuilder(mockInputs)).rejects.toThrow('Failed to resolve dockerfile path');
expect(core.warning).toHaveBeenCalledWith('Error during Blacksmith builder setup: Failed to resolve dockerfile path. Failing the build because nofallback is set.');
expect(reporter.reportBuilderCreationFailed).toHaveBeenCalledWith(new Error('Failed to resolve dockerfile path'));
expect(reporter.reportBuildPushActionFailure).toHaveBeenCalledWith(new Error('Failed to resolve dockerfile path'));
});

test('should handle error in setupStickyDisk with nofallback=false', async () => {
Expand All @@ -67,7 +67,7 @@ describe('startBlacksmithBuilder', () => {

expect(result).toEqual({addr: null, buildId: null, exposeId: ''});
expect(core.warning).toHaveBeenCalledWith('Error during Blacksmith builder setup: Failed to obtain Blacksmith builder. Falling back to a local build.');
expect(reporter.reportBuilderCreationFailed).toHaveBeenCalledWith(new Error('Failed to obtain Blacksmith builder'));
expect(reporter.reportBuildPushActionFailure).toHaveBeenCalledWith(new Error('Failed to obtain Blacksmith builder'));
});

test('should handle error in setupStickyDisk with nofallback=true', async () => {
Expand All @@ -78,7 +78,7 @@ describe('startBlacksmithBuilder', () => {

await expect(main.startBlacksmithBuilder(mockInputs)).rejects.toThrow(error);
expect(core.warning).toHaveBeenCalledWith('Error during Blacksmith builder setup: Failed to obtain Blacksmith builder. Failing the build because nofallback is set.');
expect(reporter.reportBuilderCreationFailed).toHaveBeenCalledWith(error);
expect(reporter.reportBuildPushActionFailure).toHaveBeenCalledWith(error);
});

test('should successfully start buildkitd when setup succeeds', async () => {
Expand Down Expand Up @@ -106,7 +106,7 @@ describe('startBlacksmithBuilder', () => {
});
expect(setupBuilder.startAndConfigureBuildkitd).toHaveBeenCalledWith(mockParallelism, mockDevice);
expect(core.warning).not.toHaveBeenCalled();
expect(reporter.reportBuilderCreationFailed).not.toHaveBeenCalled();
expect(reporter.reportBuildPushActionFailure).not.toHaveBeenCalled();
});

test('should handle buildkitd startup failure with nofallback=false', async () => {
Expand All @@ -126,7 +126,7 @@ describe('startBlacksmithBuilder', () => {

expect(result).toEqual({addr: null, buildId: null, exposeId: ''});
expect(core.warning).toHaveBeenCalledWith('Error during buildkitd setup: Failed to start buildkitd. Falling back to a local build.');
expect(reporter.reportBuilderCreationFailed).toHaveBeenCalled();
expect(reporter.reportBuildPushActionFailure).toHaveBeenCalled();
});

test('should throw error when buildkitd fails and nofallback is true', async () => {
Expand All @@ -144,6 +144,6 @@ describe('startBlacksmithBuilder', () => {
mockInputs.nofallback = true;
await expect(main.startBlacksmithBuilder(mockInputs)).rejects.toThrow('Failed to start buildkitd');
expect(core.warning).toHaveBeenCalledWith('Error during buildkitd setup: Failed to start buildkitd. Failing the build because nofallback is set.');
expect(reporter.reportBuilderCreationFailed).toHaveBeenCalled();
expect(reporter.reportBuildPushActionFailure).toHaveBeenCalled();
});
});
3 changes: 2 additions & 1 deletion src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export async function startBlacksmithBuilder(inputs: context.Inputs): Promise<{a
} catch (error) {
// If the builder setup fails for any reason, we check if we should fallback to a local build.
// If we should not fallback, we rethrow the error and fail the build.
await reporter.reportBuilderCreationFailed(error);
await reporter.reportBuildPushActionFailure(error);

let errorMessage = `Error during Blacksmith builder setup: ${error.message}`;
if (error.message.includes('buildkitd')) {
Expand Down Expand Up @@ -346,6 +346,7 @@ actionsToolkit.run(
}
} catch (error) {
core.warning(`Error during Blacksmith builder shutdown: ${error.message}`);
await reporter.reportBuildPushActionFailure(error);
}
}
});
Expand Down
5 changes: 3 additions & 2 deletions src/reporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ import axios, {AxiosError, AxiosInstance, AxiosResponse} from 'axios';
import {ExportRecordResponse} from '@docker/actions-toolkit/lib/types/buildx/history';
import * as utils from './utils';

export async function reportBuilderCreationFailed(error?: Error) {
export async function reportBuildPushActionFailure(error?: Error) {
const requestOptions = {
stickydisk_key: process.env.GITHUB_REPO_NAME || '',
repo_name: process.env.GITHUB_REPO_NAME || '',
region: process.env.BLACKSMITH_REGION || 'eu-central',
arch: process.env.BLACKSMITH_ENV?.includes('arm') ? 'arm64' : 'amd64',
vm_id: process.env.VM_ID || '',
petname: process.env.PETNAME || ''
petname: process.env.PETNAME || '',
message: error?.message || ''
};
const retryCondition = (error: AxiosError) => {
return error.response?.status ? error.response.status > 500 : false;
Expand Down

0 comments on commit 97dd12c

Please sign in to comment.