fix(backport): do not push if there's no differences and fix error catch

Signed-off-by: John Molakvoæ <[email protected]>

src/backport.ts

@@ -1,7 +1,7 @@
 import { existsSync, rmSync } from 'node:fs'
 import { Octokit } from '@octokit/rest'
 
-import { cherryPickCommits, cloneAndCacheRepo, hasEmptyCommits, hasSkipCiCommits, pushBranch } from './gitUtils'
+import { cherryPickCommits, cloneAndCacheRepo, hasDiff, hasEmptyCommits, hasSkipCiCommits, pushBranch } from './gitUtils'
 import { CherryPickResult, Task } from './constants'
 import { debug, error, info, warn } from './logUtils'
 import { Reaction, addReaction, getAuthToken, getAvailableLabels, getLabelsFromPR, getAvailableMilestones, requestReviewers, getReviewers, createBackportPullRequest, setPRLabels, setPRMilestone, getChangesFromPR, updatePRBody, commentOnPR, assignToPR } from './githubUtils'
@@ -27,8 +27,7 @@ export const backport = (task: Task) => new Promise<void>((resolve, reject) => {
 				tmpDir = await cloneAndCacheRepo(task, backportBranch)
 				info(task, `Cloned to ${tmpDir}`)
 			} catch (e) {
-				reject(`Failed to clone repository: ${e.message}`)
-				throw e
+				throw new Error(`Failed to clone repository: ${e.message}`)
 			}
 
 			// Cherry pick the commits
@@ -40,17 +39,20 @@ export const backport = (task: Task) => new Promise<void>((resolve, reject) => {
 					info(task, `Cherry picking commits successful`)
 				}
 			} catch (e) {
-				reject(`Failed to cherry pick commits: ${e.message}`)
-				throw e
+				throw new Error(`Failed to cherry pick commits: ${e.message}`)
+			}
+
+			// Check if there are any changes to backport
+			if (!await hasDiff(tmpDir, task.branch, backportBranch)) {
+				throw new Error(`No changes found in backport branch`)
 			}
 
 			// Push the branch
 			try {
 				await pushBranch(task, tmpDir, token)
 				info(task, `Pushed branch ${backportBranch}`)
 			} catch (e) {
-				reject(`Failed to push branch: ${e.message}`)
-				throw e
+				throw new Error(`Failed to push branch: ${e.message}`)
 			}
 
 			// Create the pull request
@@ -71,11 +73,10 @@ export const backport = (task: Task) => new Promise<void>((resolve, reject) => {
 					await requestReviewers(octokit, task, prNumber, [task.author])
 					info(task, `Requested reviews from ${[...reviewers, task.author].join(', ')}`)
 				} catch (e) {
-					reject(`Failed to request reviews: ${e.message}`)
+					throw new Error(`Failed to request reviews: ${e.message}`)
 				}
 			} catch (e) {
-				reject(`Failed to create pull request: ${e.message}`)
-				throw e
+				throw new Error(`Failed to create pull request: ${e.message}`)
 			}
 
 			// Get labels from original PR and set them on the new PR
@@ -86,7 +87,8 @@ export const backport = (task: Task) => new Promise<void>((resolve, reject) => {
 				await setPRLabels(octokit, task, prNumber, labels)
 				info(task, `Set labels: ${labels.join(', ')}`)
 			} catch (e) {
-				reject(`Failed to get labels: ${e.message}`)
+				error(task, `Failed to get and set labels: ${e.message}`)
+				// continue, this is not a fatal error
 			}
 
 			// Find new appropriate Milestone and set it on the new PR
@@ -96,15 +98,17 @@ export const backport = (task: Task) => new Promise<void>((resolve, reject) => {
 				await setPRMilestone(octokit, task, prNumber, milestone)
 				info(task, `Set milestone: ${milestone.title}`)
 			} catch (e) {
-				warn(task, `Failed to find appropriate milestone: ${e.message}`)
+				error(task, `Failed to find appropriate milestone: ${e.message}`)
+				// continue, this is not a fatal error
 			}
 
 			// Assign the PR to the author of the original PR
 			try {
 				await assignToPR(octokit, task, prNumber, [task.author])
 				info(task, `Assigned original author: ${task.author}`)
 			} catch (e) {
-				reject(`Failed to assign PR: ${e.message}`)
+				error(task, `Failed to assign PR: ${e.message}`)
+				// continue, this is not a fatal error
 			}
 
 			// Compare the original PR with the new PR
@@ -125,10 +129,12 @@ export const backport = (task: Task) => new Promise<void>((resolve, reject) => {
 						await updatePRBody(octokit, task, prNumber, newBody)
 					}
 				} catch (e) {
-					reject(`Failed to update PR body: ${e.message}`)
+					error(task, `Failed to update PR body: ${e.message}`)
+					// continue, this is not a fatal error
 				}
 			} catch (e) {
-				reject(`Failed to compare changes: ${e.message}`)
+				error(task, `Failed to compare changes: ${e.message}`)
+				// continue, this is not a fatal error
 			}
 
 			// Success! We're done here
@@ -140,8 +146,11 @@ export const backport = (task: Task) => new Promise<void>((resolve, reject) => {
 				const failureComment = getFailureCommentBody(task, backportBranch, e?.message)
 				await commentOnPR(octokit, task, failureComment)
 			} catch (e) {
-				reject(`Failed to comment failure on PR: ${e.message}`)
+				error(task, `Failed to comment failure on PR: ${e.message}`)
+				// continue, this is not a fatal error
 			}
+
+			reject(`Failed to backport: ${e.message}`)
 		}
 
 		// Remove the temp dir if it exists

src/gitUtils.ts

@@ -156,16 +156,19 @@ export const hasSkipCiCommits = async (repoRoot: string, commits: number): Promi
 	return commitMessages.some(message => message.includes('[skip ci]'))
 }
 
+export const hasDiff = async (repoRoot: string, base: string, head: string): Promise<boolean> => {
+	const git = simpleGit(repoRoot)
+	const diff = await git.raw(['diff', '--stat', base, head])
+	return diff !== ''
+}
 
 export const hasEmptyCommits = async (repoRoot: string, commits: number): Promise<boolean> => {
-	const git = simpleGit(repoRoot)
 	let hasEmptyCommits = false
 	for (let count = 0; count < commits; count++) {
-		const diff = await git.raw(['diff', '--stat', `HEAD~${count}`, `HEAD~${count + 1}`])
-		if (diff === '') {
+		if (await hasDiff(repoRoot, `HEAD~${count}`, `HEAD~${count + 1}`)) {
 			hasEmptyCommits = true
 			break
 		}
 	}
 	return hasEmptyCommits
-}
+}

src/index.ts

@@ -227,14 +227,17 @@ app.webhooks.on(['issue_comment.created'], async ({ payload }) => {
 
 			// If the PR is already merged, we can start the backport right away
 			if (isMerged) {
-				addToQueue(task).then(async () => {
+				try {
+					await addToQueue(task)
 					// Remove the backport label from the PR on success
 					try {
 						await removePRLabel(authOctokit, task, prNumber, LABEL_BACKPORT)
 					} catch (e) {
 						error(`\nFailed to remove backport label from PR ${htmlUrl}: ${e.message}`)
 					}
-				})
+				} catch (e) {
+					// Safely ignore
+				}
 			}
 		} catch (e) {
 			// This should really not happen, but if it does, we want to know about it