Skip to content

Commit

Permalink
Merge pull request #122 from vbihun/replace-promise-all-with-promise-…
Browse files Browse the repository at this point in the history
…allsettled

Replace all promise.All with promise.allSettled
  • Loading branch information
vbihun authored Sep 23, 2024
2 parents 67016bd + febad62 commit f4b625e
Show file tree
Hide file tree
Showing 22 changed files with 484 additions and 83 deletions.
9 changes: 7 additions & 2 deletions src/entry/extension-points/pre-uninstall.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { getFormattedErrors, hasRejections } from '../../utils/promise-allsettled-helpers';
import { disconnectGroup } from '../../services/disconnect-group';
import { getForgeAppId } from '../../utils/get-forge-app-id';
import { getGroupIds } from '../../utils/storage-utils';
Expand All @@ -14,10 +15,14 @@ export default async function preUninstall(payload: PreUninstallPayload): Promis
console.log(`Performing preUninstall for site ${cloudId}`);

const forgeAppId = getForgeAppId();
const groupIds = await getGroupIds();

try {
await Promise.all(groupIds.map((groupId) => disconnectGroup(groupId, cloudId, forgeAppId)));
const groupIds = await getGroupIds();

const results = await Promise.allSettled(groupIds.map((groupId) => disconnectGroup(groupId, cloudId, forgeAppId)));
if (hasRejections(results)) {
throw new Error(`Error while disconnecting groups: ${getFormattedErrors(results)}`);
}
} catch (e) {
console.error({ message: 'Error performing preUninstall', error: e });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { getTrackingBranchName } from '../../../services/get-tracking-branch';
import { MergeRequestEvent } from '../../../types';
import { insertMetricValues } from '../../../services/insert-metric-values';
import { getMRCycleTime, getOpenMergeRequestsCount } from '../../../services/compute-event-and-metrics';
import { ALL_SETTLED_STATUS, getFormattedErrors } from '../../../utils/promise-allsettled-helpers';

export const handleMergeRequestEvent = async (
event: MergeRequestEvent,
Expand All @@ -18,11 +19,26 @@ export const handleMergeRequestEvent = async (
const trackingBranch = await getTrackingBranchName(groupToken, id, defaultBranch);

if (trackingBranch === targetBranch) {
const [cycleTime, openMergeRequestsCount] = await Promise.all([
const [cycleTimeResult, openMergeRequestsCountResult] = await Promise.allSettled([
getMRCycleTime(groupToken, id, trackingBranch),
getOpenMergeRequestsCount(groupToken, id, trackingBranch),
]);

if (
cycleTimeResult.status === ALL_SETTLED_STATUS.REJECTED ||
openMergeRequestsCountResult.status === ALL_SETTLED_STATUS.REJECTED
) {
throw new Error(
`Failed to get merge request cycle time or open merge request count: ${getFormattedErrors([
cycleTimeResult,
openMergeRequestsCountResult,
])}`,
);
}

const cycleTime = cycleTimeResult.value;
const openMergeRequestsCount = openMergeRequestsCountResult.value;

const metricInput = {
projectID: id.toString(),
metrics: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { getTrackingBranchName } from '../../../services/get-tracking-branch';
import { MOCK_CLOUD_ID, TEST_TOKEN } from '../../../__tests__/fixtures/gitlab-data';
import { ComponentSyncDetails } from '../../../types';
import { EXTERNAL_SOURCE } from '../../../constants';
import { ALL_SETTLED_STATUS, getFormattedErrors } from '../../../utils/promise-allsettled-helpers';

jest.mock('../../../services/sync-component-with-file', () => {
return {
Expand All @@ -24,6 +25,10 @@ jest.mock('../../../services/get-tracking-branch');
jest.spyOn(global.console, 'error').mockImplementation(() => ({}));

const MOCK_ERROR = new Error('Unexpected Error');
const RejectedPromiseSettled: PromiseSettledResult<unknown> = {
status: ALL_SETTLED_STATUS.REJECTED,
reason: MOCK_ERROR,
};

describe('Gitlab push events', () => {
const event = generatePushEvent();
Expand Down Expand Up @@ -221,6 +226,9 @@ describe('Gitlab push events', () => {

await handlePushEvent(event, TEST_TOKEN, MOCK_CLOUD_ID);

expect(console.error).toBeCalledWith('Error while handling push event', MOCK_ERROR);
expect(console.error).toBeCalledWith(
'Error while handling push event',
new Error(`Error removing components: ${getFormattedErrors([RejectedPromiseSettled])}`),
);
});
});
14 changes: 13 additions & 1 deletion src/entry/webtriggers/gitlab-event-handlers/handle-push-event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { ComponentSyncDetails, PushEvent } from '../../../types';
import { getTrackingBranchName } from '../../../services/get-tracking-branch';
import { unlinkComponentFromFile } from '../../../client/compass';
import { EXTERNAL_SOURCE } from '../../../constants';
import { hasRejections, getFormattedErrors } from '../../../utils/promise-allsettled-helpers';

export const handlePushEvent = async (event: PushEvent, groupToken: string, cloudId: string): Promise<void> => {
try {
Expand Down Expand Up @@ -56,6 +57,12 @@ export const handlePushEvent = async (event: PushEvent, groupToken: string, clou
}),
);

const creationAndUpdateResults = await Promise.allSettled([...creates, ...updates]);

if (hasRejections(creationAndUpdateResults)) {
throw new Error(`Error creating or updating components: ${getFormattedErrors(creationAndUpdateResults)}`);
}

const removals = componentsToUnlink.map((componentToUnlink) =>
unlinkComponentFromFile({
cloudId,
Expand All @@ -67,7 +74,12 @@ export const handlePushEvent = async (event: PushEvent, groupToken: string, clou
: [],
}),
);
await Promise.all([...creates, ...updates, ...removals]);

const removalResults = await Promise.allSettled(removals);

if (hasRejections(removalResults)) {
throw new Error(`Error removing components: ${getFormattedErrors(removalResults)}`);
}
} catch (e) {
console.error('Error while handling push event', e);
}
Expand Down
11 changes: 10 additions & 1 deletion src/services/clear-storage.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { storage, ListResult, startsWith } from '@forge/api';
import { CLEAR_STORAGE_CHUNK_SIZE, CLEAR_STORAGE_DELAY, STORAGE_KEYS, STORAGE_SECRETS } from '../constants';
import { deleteKeysFromStorageByChunks } from '../utils/storage-utils';
import { getFormattedErrors, hasRejections } from '../utils/promise-allsettled-helpers';

const getLastFailedProjectsKeys = async (): Promise<string[]> => {
const lastFailedProjects: ListResult = await storage
Expand Down Expand Up @@ -45,5 +46,13 @@ export const clearImportKeys = async (): Promise<void> => {
};

export const deleteGroupDataFromStorage = async (groupId: string): Promise<void> => {
await Promise.all([clearStorageSecretsForGroup(groupId), clearStorageEntriesForGroup(groupId), clearImportKeys()]);
const deleteGroupDataResult = await Promise.allSettled([
clearStorageSecretsForGroup(groupId),
clearStorageEntriesForGroup(groupId),
clearImportKeys(),
]);

if (hasRejections(deleteGroupDataResult)) {
throw new Error(`Error deleting group data: ${getFormattedErrors(deleteGroupDataResult)}`);
}
};
26 changes: 24 additions & 2 deletions src/services/compute-event-and-metrics/get-recent-deployments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { getRecentDeployments, gitlabAPiDeploymentToCompassDataProviderDeploymen
import { getProjectEnvironments } from '../environment';
import { getDateInThePast } from '../../utils/time-utils';
import { isSendStagingEventsEnabled } from '../feature-flags';
import { getFormattedErrors, hasRejections } from '../../utils/promise-allsettled-helpers';

const newGetDeploymentsForEnvironments = async (
groupToken: string,
Expand Down Expand Up @@ -35,7 +36,17 @@ const newGetDeploymentsForEnvironments = async (
});

// combine results from multiple projectEnvironments into single array
return Promise.all(deploymentEvents).then((results) => results.flat());
const settledResults = await Promise.allSettled(deploymentEvents);

if (hasRejections(settledResults)) {
throw new Error(`Error getting deployment: ${getFormattedErrors(settledResults)}`);
}

const result = settledResults.map(
(settledResult) => (settledResult as PromiseFulfilledResult<DataProviderDeploymentEvent[]>).value,
);

return result.flat();
};

export const getDeploymentsForEnvironmentTiers = async (
Expand All @@ -62,7 +73,18 @@ export const getDeploymentsForEnvironmentTiers = async (
[],
);

const deployments = (await Promise.all(getDeploymentsPromises)).flat();
const deploymentsResult = await Promise.allSettled(getDeploymentsPromises);

if (hasRejections(deploymentsResult)) {
throw new Error(`Error getting deployments: ${getFormattedErrors(deploymentsResult)}`);
}

const deploymentsValues = deploymentsResult.map(
(deploymentResult) => (deploymentResult as PromiseFulfilledResult<Deployment[]>).value,
);

const deployments = deploymentsValues.flat();

const deploymentEvents = deployments
.map((deployment) =>
gitlabAPiDeploymentToCompassDataProviderDeploymentEvent(deployment, projectName, EnvironmentTier.PRODUCTION),
Expand Down
19 changes: 14 additions & 5 deletions src/services/data-provider-link-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { getOwnedProjectsBySearchCriteria } from '../client/gitlab';
import { STORAGE_SECRETS } from '../constants';
import { getGroupIds } from '../utils/storage-utils';
import { GitlabAPIProject } from '../types';
import { getFormattedErrors, hasRejections } from '../utils/promise-allsettled-helpers';

export const extractProjectInformation = (projectUrl: string): { projectName: string; pathName: string } | null => {
const parsedUrl = parse(projectUrl);
Expand All @@ -18,12 +19,20 @@ export const extractProjectInformation = (projectUrl: string): { projectName: st
};

export const getAllGroupTokens = async (): Promise<string[]> => {
const groupIds = await getGroupIds();
const groupTokens = await Promise.all(
groupIds.map((groupId) => storage.getSecret(`${STORAGE_SECRETS.GROUP_TOKEN_KEY_PREFIX}${groupId}`)),
);
try {
const groupIds = await getGroupIds();
const groupTokensResult = await Promise.allSettled(
groupIds.map((groupId) => storage.getSecret(`${STORAGE_SECRETS.GROUP_TOKEN_KEY_PREFIX}${groupId}`)),
);

if (hasRejections(groupTokensResult)) {
throw new Error(`Error getting group tokens ${getFormattedErrors(groupTokensResult)}`);
}

return groupTokens;
return groupTokensResult.map((groupTokenResult) => (groupTokenResult as PromiseFulfilledResult<string>).value);
} catch (e) {
throw new Error(`Error while getting all group tokens: ${e}`);
}
};

function doesURLMatch(projectUrl: string, path: string, name: string) {
Expand Down
11 changes: 10 additions & 1 deletion src/services/deployment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { fetchPaginatedData } from '../utils/fetchPaginatedData';
import { getProjectEnvironments } from './environment';
import { isSendStagingEventsEnabled } from './feature-flags';
import { truncateProjectNameString } from '../utils/event-mapping';
import { getFormattedErrors, hasRejections } from '../utils/promise-allsettled-helpers';

export const gitLabStateToCompassFormat = (state: string): CompassDeploymentEventState => {
switch (state) {
Expand Down Expand Up @@ -153,7 +154,15 @@ export const getDeploymentAfter28Days = async (
[],
);

const promisesResponse = await Promise.all(getDeploymentsPromises);
const promisesResponseResults = await Promise.allSettled(getDeploymentsPromises);

if (hasRejections(promisesResponseResults)) {
throw new Error(`Error getting deployments ${getFormattedErrors(promisesResponseResults)}`);
}

const promisesResponse = promisesResponseResults.map(
(promisesResponseResult) => (promisesResponseResult as PromiseFulfilledResult<{ data: Deployment[] }>).value,
);

return promisesResponse ? promisesResponse.map((deployment) => deployment.data).flat() : [];
};
Expand Down
16 changes: 14 additions & 2 deletions src/services/fetch-projects.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
sortedProjects,
} from '../__tests__/helpers/gitlab-helper';
import { MergeRequest } from '../types';
import { ALL_SETTLED_STATUS, getFormattedErrors } from '../utils/promise-allsettled-helpers';

jest.mock('../client/gitlab');
jest.mock('../client/compass');
Expand Down Expand Up @@ -60,6 +61,13 @@ const mergeRequestMock: MergeRequest[] = [
},
];

const MOCK_ERROR = 'Error: Error while getting repository additional fields.';

const RejectedPromiseSettled: PromiseSettledResult<unknown> = {
status: ALL_SETTLED_STATUS.REJECTED,
reason: MOCK_ERROR,
};

describe('Fetch Projects Service', () => {
beforeEach(() => {
jest.clearAllMocks();
Expand Down Expand Up @@ -128,15 +136,19 @@ describe('Fetch Projects Service', () => {
mockGetProjects.mockRejectedValue(undefined);

await expect(getGroupProjects(MOCK_CLOUD_ID, MOCK_GROUP_ID, 1, 1)).rejects.toThrow(
new Error('Error while fetching group projects from Gitlab!'),
`Error while getting group projects: ${new Error('Error while fetching group projects from Gitlab!')}`,
);
});

it('returns error in case when getComponentByExternalAlias fails', async () => {
mockGetComponentByExternalAlias.mockRejectedValue(undefined);

const settledError = new Error(
`Error checking project with existing components: ${getFormattedErrors([RejectedPromiseSettled])}`,
);

await expect(getGroupProjects(MOCK_CLOUD_ID, MOCK_GROUP_ID, 1, 1)).rejects.toThrow(
new Error('Error: Error while getting repository additional fields.'),
`Error while getting group projects: ${settledError}`,
);
});

Expand Down
Loading

0 comments on commit f4b625e

Please sign in to comment.