fix: race condition with check runs (#283)

src/index.ts

@@ -110,19 +110,19 @@ const probotHandler: ApplicationFunction = async (robot, { getRouter }) => {
       if (!label.name.startsWith(PRStatus.TARGET)) continue;
       const targetBranch = labelToTargetBranch(label, PRStatus.TARGET);
       const runName = `${CHECK_PREFIX}${targetBranch}`;
-      const existing = checkRuns.find((run) => run.name === runName);
-      if (existing) {
-        if (existing.conclusion !== 'neutral') continue;
+      let checkRun = checkRuns.find((run) => run.name === runName);
+      if (checkRun) {
+        if (checkRun.conclusion !== 'neutral') continue;
 
         await context.octokit.checks.update(
           context.repo({
-            name: existing.name,
-            check_run_id: existing.id,
+            name: checkRun.name,
+            check_run_id: checkRun.id,
             status: 'queued' as 'queued',
           }),
         );
       } else {
-        await context.octokit.checks.create(
+        const response = await context.octokit.checks.create(
           context.repo({
             name: runName,
             head_sha: pr.head.sha,
@@ -130,6 +130,8 @@ const probotHandler: ApplicationFunction = async (robot, { getRouter }) => {
             details_url: 'https://github.com/electron/trop',
           }),
         );
+
+        checkRun = response.data;
       }
 
       await backportImpl(
@@ -138,6 +140,7 @@ const probotHandler: ApplicationFunction = async (robot, { getRouter }) => {
         pr,
         targetBranch,
         BackportPurpose.Check,
+        checkRun,
       );
     }
 

src/operations/backport-to-location.ts

@@ -12,7 +12,7 @@ const createOrUpdateCheckRun = async (
   pr: WebHookPR,
   targetBranch: string,
 ) => {
-  const check = await getCheckRun(context, pr, targetBranch);
+  let check = await getCheckRun(context, pr, targetBranch);
 
   if (check) {
     if (check.conclusion === 'neutral') {
@@ -25,7 +25,7 @@ const createOrUpdateCheckRun = async (
       );
     }
   } else {
-    await context.octokit.checks.create(
+    const response = await context.octokit.checks.create(
       context.repo({
         name: `${CHECK_PREFIX}${targetBranch}`,
         head_sha: pr.head.sha,
@@ -33,7 +33,11 @@ const createOrUpdateCheckRun = async (
         details_url: 'https://github.com/electron/trop',
       }),
     );
+
+    check = response.data;
   }
+
+  return check;
 };
 
 /**
@@ -74,7 +78,7 @@ export const backportToLabel = async (
     return;
   }
 
-  await createOrUpdateCheckRun(context, pr, targetBranch);
+  const checkRun = await createOrUpdateCheckRun(context, pr, targetBranch);
 
   const labelToRemove = label.name;
   const labelToAdd = label.name.replace(PRStatus.TARGET, PRStatus.IN_FLIGHT);
@@ -84,6 +88,7 @@ export const backportToLabel = async (
     pr,
     targetBranch,
     BackportPurpose.ExecuteBackport,
+    checkRun,
     labelToRemove,
     labelToAdd,
   );
@@ -108,7 +113,7 @@ export const backportToBranch = async (
     `Executing backport to branch '${targetBranch}'`,
   );
 
-  await createOrUpdateCheckRun(context, pr, targetBranch);
+  const checkRun = await createOrUpdateCheckRun(context, pr, targetBranch);
 
   const labelToRemove = undefined;
   const labelToAdd = PRStatus.IN_FLIGHT + targetBranch;
@@ -118,6 +123,7 @@ export const backportToBranch = async (
     pr,
     targetBranch,
     BackportPurpose.ExecuteBackport,
+    checkRun,
     labelToRemove,
     labelToAdd,
   );

src/utils.ts

@@ -450,6 +450,7 @@ export const backportImpl = async (
   pr: WebHookPR,
   targetBranch: string,
   purpose: BackportPurpose,
+  checkRun: NonNullable<Awaited<ReturnType<typeof getCheckRun>>>,
   labelToRemove?: string,
   labelToAdd?: string,
 ) => {
@@ -485,16 +486,13 @@ export const backportImpl = async (
     `backport-${pr.head.sha}-${targetBranch}-${purpose}`,
     async () => {
       log('backportImpl', LogLevel.INFO, `Executing ${bp} for "${slug}"`);
-      const checkRun = await getCheckRun(context, pr, targetBranch);
-      if (checkRun) {
-        await context.octokit.checks.update(
-          context.repo({
-            check_run_id: checkRun.id,
-            name: checkRun.name,
-            status: 'in_progress' as 'in_progress',
-          }),
-        );
-      }
+      await context.octokit.checks.update(
+        context.repo({
+          check_run_id: checkRun.id,
+          name: checkRun.name,
+          status: 'in_progress' as 'in_progress',
+        }),
+      );
 
       const repoAccessToken = await getRepoToken(robot, context);
 
@@ -679,20 +677,18 @@ export const backportImpl = async (
         log('backportImpl', LogLevel.INFO, 'Backport process complete');
       }
 
-      if (checkRun) {
-        context.octokit.checks.update(
-          context.repo({
-            check_run_id: checkRun.id,
-            name: checkRun.name,
-            conclusion: 'success' as 'success',
-            completed_at: new Date().toISOString(),
-            output: {
-              title: 'Clean Backport',
-              summary: `This PR was checked and can be backported to "${targetBranch}" cleanly.`,
-            },
-          }),
-        );
-      }
+      context.octokit.checks.update(
+        context.repo({
+          check_run_id: checkRun.id,
+          name: checkRun.name,
+          conclusion: 'success' as 'success',
+          completed_at: new Date().toISOString(),
+          output: {
+            title: 'Clean Backport',
+            summary: `This PR was checked and can be backported to "${targetBranch}" cleanly.`,
+          },
+        }),
+      );
 
       await fs.remove(createdDir);
     },
@@ -759,30 +755,27 @@ export const backportImpl = async (
         ]);
       }
 
-      const checkRun = await getCheckRun(context, pr, targetBranch);
-      if (checkRun) {
-        const mdSep = '``````````````````````````````';
-        const updateOpts = context.repo({
-          check_run_id: checkRun.id,
-          name: checkRun.name,
-          conclusion: 'neutral' as 'neutral',
-          completed_at: new Date().toISOString(),
-          output: {
-            title: 'Backport Failed',
-            summary: `This PR was checked and could not be automatically backported to "${targetBranch}" cleanly`,
-            text: diff
-              ? `Failed Diff:\n\n${mdSep}diff\n${rawDiff}\n${mdSep}`
-              : undefined,
-            annotations: annotations ? annotations : undefined,
-          },
-        });
-        try {
-          await context.octokit.checks.update(updateOpts);
-        } catch (err) {
-          // A GitHub error occurred - try to mark it as a failure without annotations.
-          updateOpts.output!.annotations = undefined;
-          await context.octokit.checks.update(updateOpts);
-        }
+      const mdSep = '``````````````````````````````';
+      const updateOpts = context.repo({
+        check_run_id: checkRun.id,
+        name: checkRun.name,
+        conclusion: 'neutral' as 'neutral',
+        completed_at: new Date().toISOString(),
+        output: {
+          title: 'Backport Failed',
+          summary: `This PR was checked and could not be automatically backported to "${targetBranch}" cleanly`,
+          text: diff
+            ? `Failed Diff:\n\n${mdSep}diff\n${rawDiff}\n${mdSep}`
+            : undefined,
+          annotations: annotations ? annotations : undefined,
+        },
+      });
+      try {
+        await context.octokit.checks.update(updateOpts);
+      } catch (err) {
+        // A GitHub error occurred - try to mark it as a failure without annotations.
+        updateOpts.output!.annotations = undefined;
+        await context.octokit.checks.update(updateOpts);
       }
     },
   );