Skip to content

Commit

Permalink
Merge pull request #384 from snyk-tech-services/fix/break-sync-early
Browse files Browse the repository at this point in the history
fix: break early if a whole batch fails to sync
  • Loading branch information
lili2311 authored Nov 16, 2022
2 parents 799ab91 + 640f7b7 commit ad2b540
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/cmds/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export async function syncOrg(
res.meta.projects.failed.length == 0;
const orgMessage = nothingToUpdate
? `Did not detect any changes to apply`
: `Processed ${res.processedTargets} targets\nUpdated ${res.meta.projects.updated.length} projects\n${res.meta.projects.failed.length} projects failed to update\nFind more information in ${res.fileName} and ${res.failedFileName}`;
: `Processed ${res.processedTargets} targets (${res.failedTargets} failed)\nUpdated ${res.meta.projects.updated.length} projects\n${res.meta.projects.failed.length} projects failed to update\nFind more information in ${res.fileName} and ${res.failedFileName}`;

return {
fileName: res.fileName,
Expand Down
23 changes: 22 additions & 1 deletion src/scripts/sync/sync-org-projects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export async function updateOrgTargets(
fileName: string;
failedFileName: string;
processedTargets: number;
failedTargets: number;
meta: {
projects: {
updated: ProjectUpdate[];
Expand All @@ -48,6 +49,7 @@ export async function updateOrgTargets(
};
}> {
const res: {
failedTargets: number;
processedTargets: number;
meta: {
projects: {
Expand All @@ -56,6 +58,7 @@ export async function updateOrgTargets(
};
};
} = {
failedTargets: 0,
processedTargets: 0,
meta: {
projects: {
Expand Down Expand Up @@ -128,6 +131,7 @@ export async function updateOrgTargets(
sourceUrl,
);
res.processedTargets += response.processedTargets;
res.failedTargets += response.failedTargets;
res.meta.projects.updated.push(...response.meta.projects.updated);
res.meta.projects.failed.push(...response.meta.projects.failed);
},
Expand Down Expand Up @@ -159,6 +163,7 @@ export async function updateTargets(
dryRun = false,
sourceUrl?: string,
): Promise<{
failedTargets: number;
processedTargets: number;
meta: {
projects: {
Expand All @@ -168,15 +173,18 @@ export async function updateTargets(
};
}> {
let processedTargets = 0;
let failedTargets = 0;
const updatedProjects: ProjectUpdate[] = [];
const failedProjects: ProjectUpdateFailure[] = [];

const loggingPath = getLoggingPath();
const concurrentTargets = 50;

await pMap(
targets,
async (target: SnykTarget) => {
try {
// TODO: is target reachable via SCM token? If not skip listing projects
const { updated, failed } = await syncProjectsForTarget(
requestManager,
orgId,
Expand All @@ -195,19 +203,32 @@ export async function updateTargets(
await logFailedToUpdateProjects(orgId, failed);
}
} catch (e) {
failedTargets += 1;
debug(e);
const errorMessage: string = e.message;
console.warn(
`Failed to sync target ${target.attributes.displayName}. ERROR: ${errorMessage}`,
);
await logFailedSync(orgId, target, errorMessage, loggingPath);
}

if (
updatedProjects.length == 0 &&
processedTargets == concurrentTargets
) {
console.error(
`Every target in this batch failed to update projects, stopping as this is unexpected! Please check if everything is configured ok and review the logs located at ${loggingPath}`,
);
// die immediately
process.exit(1);
}
},
{ concurrency: 20 },
{ concurrency: concurrentTargets },
);
return {
processedTargets,
// TODO: collect failed targets & log them with reason?
failedTargets,
meta: {
projects: {
updated: updatedProjects,
Expand Down
2 changes: 1 addition & 1 deletion src/scripts/sync/sync-projects-per-target.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ export async function bulkUpdateProjectsBranch(
});
}
},
{ concurrency: 30 },
{ concurrency: 50 },
);

return { updated: updatedProjects, failed: failedProjects };
Expand Down
8 changes: 8 additions & 0 deletions test/scripts/sync/sync-org-projects.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ describe('updateTargets', () => {

// Assert
expect(res).toStrictEqual({
failedTargets: 0,
processedTargets: 1,
meta: {
projects: {
Expand Down Expand Up @@ -185,6 +186,7 @@ describe('updateTargets', () => {

// Assert
expect(res).toStrictEqual({
failedTargets: 0,
processedTargets: 1,
meta: {
projects: {
Expand Down Expand Up @@ -283,6 +285,7 @@ describe('updateTargets', () => {
// Assert
expect(res).toStrictEqual({
processedTargets: 1,
failedTargets: 0,
meta: {
projects: {
updated: updated.map((u) => ({ ...u, target: testTargets[0] })),
Expand Down Expand Up @@ -386,6 +389,7 @@ describe('updateTargets', () => {

// Assert
expect(res).toStrictEqual({
failedTargets: 0,
processedTargets: 1,
meta: {
projects: {
Expand Down Expand Up @@ -492,6 +496,7 @@ describe('updateOrgTargets', () => {
},
},
processedTargets: 0,
failedTargets: 1,
});
});
});
Expand Down Expand Up @@ -585,6 +590,7 @@ describe('updateOrgTargets', () => {
},
},
processedTargets: 0,
failedTargets: 1,
});
});

Expand Down Expand Up @@ -682,6 +688,7 @@ describe('updateOrgTargets', () => {
},
},
processedTargets: 2,
failedTargets: 0,
});
expect(featureFlagsSpy).toHaveBeenCalledTimes(1);
expect(listTargetsSpy).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -826,6 +833,7 @@ describe('updateOrgTargets', () => {
},
},
processedTargets: 2,
failedTargets: 0,
});
expect(featureFlagsSpy).toHaveBeenCalledTimes(1);
expect(listTargetsSpy).toHaveBeenCalledTimes(1);
Expand Down

0 comments on commit ad2b540

Please sign in to comment.