improve error handling so that the bot is able to communicate errors …

…better in comments

src/cmd.rs

@@ -148,7 +148,7 @@ fn handle_cmd_result<'a>(
 									err_output.replace(secret, "${SECRET}");
 							}
 						}
-						log::error!(
+						log::info!(
 							"handle_cmd_result: {} failed with error: {}",
 							cmd_display,
 							err_output

src/companion.rs

@@ -14,8 +14,11 @@ use crate::{
 	error::*,
 	github::*,
 	github_bot::GithubBot,
-	webhook::{merge, ready_to_merge, wait_to_merge},
-	Result, COMPANION_LONG_REGEX, COMPANION_PREFIX_REGEX,
+	webhook::{
+		get_latest_checks_state, get_latest_statuses_state, merge,
+		ready_to_merge, wait_to_merge,
+	},
+	Result, Status, COMPANION_LONG_REGEX, COMPANION_PREFIX_REGEX,
 	COMPANION_SHORT_REGEX, PR_HTML_URL_REGEX,
 };
 
@@ -340,8 +343,9 @@ pub async fn check_all_companions_are_mergeable(
 			for (html_url, owner, repo, number) in parse_all_companions(body) {
 				let companion =
 					github_bot.pull_request(&owner, &repo, number).await?;
+				let head_sha = companion.head_sha()?;
 
-				let is_owner_a_user = companion
+				let has_user_owner = companion
 					.user
 					.as_ref()
 					.map(|user| {
@@ -351,7 +355,7 @@ pub async fn check_all_companions_are_mergeable(
 							.unwrap_or(false)
 					})
 					.unwrap_or(false);
-				if !is_owner_a_user {
+				if !has_user_owner {
 					return Err(Error::Message {
 						msg: format!(
 							"Companion {} is not owned by a user, therefore processbot would not be able to push the lockfile update to their branch due to a Github limitation (https://github.com/isaacs/github/issues/1681)",
@@ -372,6 +376,86 @@ pub async fn check_all_companions_are_mergeable(
 						),
 					});
 				}
+
+				let target_branch = github_bot
+					.branch(
+						&companion.base.repo.owner.login,
+						&companion.base.repo.name,
+						&companion.base.ref_field,
+					)
+					.await?;
+
+				let statuses = get_latest_statuses_state(
+					github_bot,
+					&owner,
+					&repo,
+					head_sha,
+					&pr.html_url,
+					Some(
+						&target_branch
+							.protection
+							.required_status_checks
+							.contexts,
+					),
+				)
+				.await?;
+				match statuses {
+					Status::Success => (),
+					Status::Pending => {
+						return Err(Error::InvalidCompanionStatus {
+							status: Status::Pending,
+							msg: format!(
+								"Companion {} has pending required statuses",
+								html_url
+							),
+						});
+					}
+					Status::Failure => {
+						return Err(Error::InvalidCompanionStatus {
+							status: Status::Failure,
+							msg: format!(
+								"Companion {} has failed required statuses",
+								html_url
+							),
+						});
+					}
+				};
+
+				let checks = get_latest_checks_state(
+					github_bot,
+					&owner,
+					&repo,
+					&head_sha,
+					&pr.html_url,
+					Some(
+						&target_branch
+							.protection
+							.required_status_checks
+							.contexts,
+					),
+				)
+				.await?;
+				match checks {
+					Status::Success => (),
+					Status::Pending => {
+						return Err(Error::InvalidCompanionStatus {
+							status: checks,
+							msg: format!(
+								"Companion {} has pending required checks",
+								html_url
+							),
+						});
+					}
+					Status::Failure => {
+						return Err(Error::InvalidCompanionStatus {
+							status: checks,
+							msg: format!(
+								"Companion {} has failed required checks",
+								html_url
+							),
+						});
+					}
+				};
 			}
 		}
 	}
@@ -447,12 +531,13 @@ async fn update_then_merge_companion(
 				&format!("parity-processbot[bot]"),
 				&updated_sha,
 				db,
+				None,
 			)
 			.await?;
 		}
 	} else {
 		return Err(Error::Message {
-			msg: "Companion PR is missing some API data.".to_string(),
+			msg: format!("Companion {} is missing some API data", html_url),
 		});
 	}
 

src/error.rs

@@ -1,3 +1,4 @@
+use crate::Status;
 use snafu::Snafu;
 
 // TODO this really should be struct { repository, owner, number }
@@ -50,11 +51,6 @@ pub enum Error {
 		actual: String,
 	},
 
-	#[snafu(display("Error getting process info: {}", source))]
-	ProcessFile {
-		source: Box<Error>,
-	},
-
 	#[snafu(display("Missing process info."))]
 	ProcessInfo {
 		errors: Option<Vec<String>>,
@@ -195,6 +191,12 @@ pub enum Error {
 	MergeFailureWillBeSolvedLater {
 		msg: String,
 	},
+
+	#[snafu(display("{}", msg))]
+	InvalidCompanionStatus {
+		status: Status,
+		msg: String,
+	},
 }
 
 impl Error {
@@ -207,6 +209,15 @@ impl Error {
 			},
 		}
 	}
+	pub fn stops_merge_attempt(&self) -> bool {
+		match self {
+			Self::WithIssue { source, .. } | Self::Merge { source, .. } => {
+				source.stops_merge_attempt()
+			}
+			Self::MergeFailureWillBeSolvedLater { .. } => false,
+			_ => true,
+		}
+	}
 }
 
 impl From<curl::Error> for Error {

src/github.rs

@@ -1,5 +1,6 @@
 use crate::{
-	error::*, utils::parse_bot_comment_from_text, Result, PR_HTML_URL_REGEX,
+	error::*, utils::parse_bot_comment_from_text,
+	PlaceholderDeserializationItem, Result, PR_HTML_URL_REGEX,
 };
 use regex::Regex;
 use serde::{Deserialize, Serialize};
@@ -9,6 +10,19 @@ pub trait HasIssueDetails {
 	fn get_issue_details(&self) -> Option<IssueDetails>;
 }
 
+#[derive(Default, Debug, Clone, PartialEq, Serialize, Deserialize)]
+pub struct BranchProtectionRequiredStatusChecks {
+	pub contexts: Vec<String>,
+}
+#[derive(Default, Debug, Clone, PartialEq, Serialize, Deserialize)]
+pub struct BranchProtection {
+	pub required_status_checks: BranchProtectionRequiredStatusChecks,
+}
+#[derive(Default, Debug, Clone, PartialEq, Serialize, Deserialize)]
+pub struct Branch {
+	pub protection: BranchProtection,
+}
+
 #[derive(Default, Debug, Clone, PartialEq, Serialize, Deserialize)]
 pub struct PullRequest {
 	pub url: String,
@@ -73,7 +87,7 @@ pub struct Issue {
 	// User might be missing when it has been deleted
 	pub user: Option<User>,
 	pub body: Option<String>,
-	pub pull_request: Option<IssuePullRequest>,
+	pub pull_request: Option<PlaceholderDeserializationItem>,
 	pub repository: Option<Repository>,
 	pub repository_url: Option<String>,
 }
@@ -172,9 +186,6 @@ pub struct Team {
 	pub id: i64,
 }
 
-#[derive(Default, Debug, Clone, PartialEq, Serialize, Deserialize)]
-pub struct IssuePullRequest {}
-
 #[derive(Default, Debug, Clone, PartialEq, Serialize, Deserialize)]
 pub struct Head {
 	pub label: Option<String>,
@@ -188,10 +199,8 @@ pub struct Head {
 #[derive(Default, Debug, Clone, PartialEq, Serialize, Deserialize)]
 pub struct Base {
 	#[serde(rename = "ref")]
-	pub ref_field: Option<String>,
-	pub sha: Option<String>,
-	// Repository might be missing when it has been deleted
-	pub repo: Option<HeadRepo>,
+	pub ref_field: String,
+	pub repo: BaseRepo,
 }
 
 #[derive(Default, Debug, Clone, PartialEq, Serialize, Deserialize)]
@@ -324,6 +333,12 @@ pub struct HeadRepo {
 	pub owner: Option<User>,
 }
 
+#[derive(Default, Debug, Clone, PartialEq, Serialize, Deserialize)]
+pub struct BaseRepo {
+	pub name: String,
+	pub owner: User,
+}
+
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
 #[serde(rename_all = "snake_case")]
 pub enum CheckRunConclusion {
@@ -354,7 +369,7 @@ pub struct WebhookIssueComment {
 	pub number: i64,
 	pub html_url: String,
 	pub repository_url: Option<String>,
-	pub pull_request: Option<IssuePullRequest>,
+	pub pull_request: Option<PlaceholderDeserializationItem>,
 }
 
 impl HasIssueDetails for WebhookIssueComment {

src/github_bot/repository.rs

@@ -3,7 +3,6 @@ use crate::{github, Result};
 use super::GithubBot;
 
 impl GithubBot {
-	/// Returns a repository with the given name.
 	pub async fn repository<A>(
 		&self,
 		owner: &str,
@@ -20,6 +19,22 @@ impl GithubBot {
 		);
 		self.client.get(url).await
 	}
+
+	pub async fn branch(
+		&self,
+		owner: &str,
+		repo: &str,
+		branch: &str,
+	) -> Result<github::Branch> {
+		let url = format!(
+			"{}/repos/{}/{}/branches/{}",
+			Self::BASE_URL,
+			owner,
+			repo,
+			branch
+		);
+		self.client.get(url).await
+	}
 }
 
 /*

src/lib.rs

@@ -1,3 +1,5 @@
+use serde::{Deserialize, Serialize};
+
 mod auth;
 pub mod bamboo;
 pub mod cmd;
@@ -24,6 +26,7 @@ pub mod webhook;
 
 pub type Result<T, E = error::Error> = std::result::Result<T, E>;
 
+#[derive(Debug)]
 pub enum Status {
 	Success,
 	Pending,
@@ -43,3 +46,12 @@ pub enum CommentCommand {
 	BurninRequest,
 	CompareReleaseRequest,
 }
+
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+pub struct PlaceholderDeserializationItem {}
+
+pub enum MergeCancelOutcome {
+	ShaDidNotExist,
+	WasCancelled,
+	WasNotCancelled,
+}