Skip to content

Commit

Permalink
Add more informative error context (#27)
Browse files Browse the repository at this point in the history
  • Loading branch information
willcrichton authored Sep 24, 2024
1 parent adde38b commit e101a6e
Show file tree
Hide file tree
Showing 5 changed files with 185 additions and 82 deletions.
2 changes: 1 addition & 1 deletion rs/crates/repo-quest/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ async fn refresh_state(quest: State<'_, Arc<Quest>>) -> Result<(), String> {
#[specta::specta]
async fn hard_reset(quest: State<'_, Arc<Quest>>, stage: u32) -> Result<(), String> {
let stage = usize::try_from(stage).unwrap();
fmt_err(quest.hard_reset(stage).await)?;
fmt_err(quest.skip_to_stage(stage).await)?;
Ok(())
}

Expand Down
60 changes: 34 additions & 26 deletions rs/crates/rq-core/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,19 @@ macro_rules! git {
($self:expr, $($arg:tt)*) => {{
let arg = format!($($arg)*);
$self.git(|cmd| {
tracing::debug!($($arg)*);
tracing::debug!("git: {arg}");
cmd.args(shlex::split(&arg).unwrap());
}).with_context(|| arg)
}).with_context(|| format!("git failed: {arg}"))
}}
}

macro_rules! git_output {
($self:expr, $($arg:tt)*) => {{
let arg = format!($($arg)*);
$self.git_output(|cmd| {
tracing::debug!("git: {arg}");
cmd.args(shlex::split(&format!($($arg)*)).unwrap());
})
}).with_context(|| format!("git failed: {arg}"))
}}
}

Expand Down Expand Up @@ -87,9 +89,8 @@ impl GitRepo {

pub fn setup_upstream(&self, upstream: &GithubRepo) -> Result<()> {
let remote = upstream.remote(GitProtocol::Https);
git!(self, "remote add {UPSTREAM} {remote}",)
.with_context(|| format!("Failed to add upstream {remote}",))?;
git!(self, "fetch {UPSTREAM}").context("Failed to fetch upstream")?;
git!(self, "remote add {UPSTREAM} {remote}")?;
git!(self, "fetch {UPSTREAM}")?;
Ok(())
}

Expand Down Expand Up @@ -152,13 +153,11 @@ impl GitRepo {
git!(self, "cherry-pick --abort").context("Failed to abort cherry-pick")?;

let upstream_target = format!("{UPSTREAM}/{target_branch}");
git!(self, "reset --hard {upstream_target}")
.with_context(|| format!("Failed to hard reset to {upstream_target}"))?;
git!(self, "reset --hard {upstream_target}")?;

git!(self, "reset --soft main").context("Failed to soft reset to main")?;

git!(self, "commit -m 'Override with reference solution'")
.context("Failed to commit reference solution")?;
git!(self, "commit -m 'Override with reference solution'")?;

MergeType::SolutionReset
}
Expand All @@ -171,23 +170,22 @@ impl GitRepo {
base_branch: &str,
target_branch: &str,
) -> Result<(String, MergeType)> {
git!(self, "checkout -b {target_branch}")
.with_context(|| format!("Failed to checkout branch {target_branch}"))?;
git!(self, "checkout -b {target_branch}")?;

let merge_type = template.apply_patch(self, base_branch, target_branch)?;

git!(self, "push -u origin {target_branch}")
.with_context(|| format!("Failed to push branch {target_branch}"))?;
git!(self, "push -u origin {target_branch}")?;

let head = self.head_commit()?;

git!(self, "checkout main").context("Failed to checkout main")?;
git!(self, "checkout main")?;

Ok((head, merge_type))
}

pub fn checkout_main_and_pull(&self) -> Result<()> {
git!(self, "checkout main").context("Failed to checkout main")?;
git!(self, "pull").context("Failed to pull main")?;
git!(self, "checkout main")?;
git!(self, "pull")?;
Ok(())
}

Expand All @@ -204,12 +202,10 @@ impl GitRepo {

pub fn diff(&self, base: &str, head: &str) -> Result<String> {
git_output!(self, "diff {base}..{head}")
.with_context(|| format!("Failed to `git diff {base}..{head}"))
}

pub fn show(&self, branch: &str, file: &str) -> Result<String> {
git_output!(self, "show {branch}:{file}")
.with_context(|| format!("Failed to `git show {branch}:{file}"))
}

pub fn show_bin(&self, branch: &str, file: &str) -> Result<Vec<u8>> {
Expand Down Expand Up @@ -250,12 +246,18 @@ impl GitRepo {
use std::os::unix::fs::PermissionsExt;
let hooks_dir = self.path.join(".githooks");
if hooks_dir.exists() {
let hooks = fs::read_dir(&hooks_dir)?;
let hooks = fs::read_dir(&hooks_dir)
.with_context(|| format!("Failed to read hooks directory: {}", hooks_dir.display()))?;
for hook in hooks {
let hook = hook?;
let mut perms = hook.metadata()?.permissions();
let hook = hook.context("Failed to read hooks directory entry")?;
let mut perms = hook
.metadata()
.with_context(|| format!("Failed to read hook metadata: {}", hook.path().display()))?
.permissions();
perms.set_mode(perms.mode() | 0o111);
fs::set_permissions(hook.path(), perms)?;
fs::set_permissions(hook.path(), perms).with_context(|| {
format!("Failed to set hook permissions: {}", hook.path().display())
})?;
}
}
}
Expand All @@ -267,10 +269,16 @@ impl GitRepo {

git!(self, "checkout -b meta")?;

let config_str = toml::to_string_pretty(&package.config)?;
fs::write(self.path.join("rqst.toml"), config_str)?;
let config_str =
toml::to_string_pretty(&package.config).context("Failed to parse package config")?;
let toml_path = self.path.join("rqst.toml");
fs::write(&toml_path, config_str)
.with_context(|| format!("Failed to write TOML to: {}", toml_path.display()))?;

package.save(&self.path.join("package.json.gz"))?;
let pkg_path = self.path.join("package.json.gz");
package
.save(&pkg_path)
.with_context(|| format!("Failed to write package to: {}", pkg_path.display()))?;

git!(self, "add .")?;
git!(self, "commit -m 'Add meta'")?;
Expand Down
94 changes: 72 additions & 22 deletions rs/crates/rq-core/src/github.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ impl GithubRepo {
.pr_handler()
.list_comments(Some(pr.number))
.send()
.await?;
.await
.with_context(|| format!("Failed to fetch comments for PR {}", pr.number))?;
let comments = comment_pages.into_iter().collect::<Vec<_>>();
Ok::<_, anyhow::Error>(FullPullRequest { data: pr, comments })
}))
Expand Down Expand Up @@ -197,11 +198,15 @@ impl GithubRepo {

pub fn clone(&self, path: &Path) -> Result<GitRepo> {
let remote = self.remote(GitProtocol::Ssh);
let status = Command::new("git")
let output = Command::new("git")
.args(["clone", &remote])
.current_dir(path)
.status()?;
ensure!(status.success(), "`git clone {remote}` failed");
.output()?;
ensure!(
output.status.success(),
"`git clone {remote}` failed, stderr:\n{}",
String::from_utf8(output.stderr)?
);
let repo = GitRepo::new(&path.join(&self.name));
Ok(repo)
}
Expand Down Expand Up @@ -238,7 +243,8 @@ impl GithubRepo {
label.description.as_deref().unwrap_or(""),
)
}))
.await?;
.await
.context("Failed to create labels")?;
Ok(())
}

Expand All @@ -259,7 +265,7 @@ impl GithubRepo {
}

pub async fn instantiate_from_package(package: &QuestPackage) -> Result<GithubRepo> {
let user = load_user().await?;
let user = load_user().await.context("Failed to load user")?;
let params = json!({
"name": &package.config.repo,
});
Expand All @@ -268,9 +274,18 @@ impl GithubRepo {
.await
.context("Failed to create repo")?;
let repo = GithubRepo::new(&user, &package.config.repo);
repo.wait_for_content(TestRepoResult::NoContent).await?;
repo.unsubscribe().await?;
repo.create_labels(&package.labels).await?;
repo
.wait_for_content(TestRepoResult::NoContent)
.await
.context("Github repo was not properly initialized")?;
repo
.unsubscribe()
.await
.context("Failed to unsubscribe from repo")?;
repo
.create_labels(&package.labels)
.await
.context("Failed to transfer package labels to repo")?;
Ok(repo)
}

Expand All @@ -287,15 +302,29 @@ impl GithubRepo {
.with_context(|| format!("Failed to clone template repo {}/{}", base.user, base.name))?;

let repo = GithubRepo::new(&user, name);
repo.wait_for_content(TestRepoResult::HasContent).await?;
repo
.wait_for_content(TestRepoResult::HasContent)
.await
.context("Github repo was not properly initialized")?;

// Unsubscribe from repo notifications to avoid annoying emails.
repo.unsubscribe().await?;
repo
.unsubscribe()
.await
.context("Failed to unsubscribe from repo")?;

// Copy all issue labels.
let mut page = base.issue_handler().list_labels_for_repo().send().await?;
let mut page = base
.issue_handler()
.list_labels_for_repo()
.send()
.await
.context("Failed to fetch labels from upstream repo")?;
let labels = page.take_items();
repo.create_labels(&labels).await?;
repo
.create_labels(&labels)
.await
.context("Failed to transfer upstream labels to repo")?;

Ok(repo)
}
Expand All @@ -305,7 +334,12 @@ impl GithubRepo {
}

pub async fn branches(&self) -> Result<Vec<Branch>> {
let pages = self.repo_handler().list_branches().send().await?;
let pages = self
.repo_handler()
.list_branches()
.send()
.await
.context("Failed to fetch branches")?;
let branches = pages.into_iter().collect::<Vec<_>>();
Ok(branches)
}
Expand Down Expand Up @@ -384,7 +418,7 @@ Note: due to a merge conflict, this PR is a hard reset to the starter code, and
"main", // don't copy base
)
.body(body);
let self_pr = request.send().await?;
let self_pr = request.send().await.context("Failed to create new PR")?;

// TODO: lots of parallelism below we should exploit

Expand All @@ -401,10 +435,14 @@ Note: due to a merge conflict, this PR is a hard reset to the starter code, and
self
.issue_handler()
.add_labels(self_pr.number, &labels)
.await?;
.await
.context("Failed to add labels to PR")?;

for comment in &pr.comments {
self.copy_pr_comment(self_pr.number, comment, head).await?;
self
.copy_pr_comment(self_pr.number, comment, head)
.await
.context("Failed to add comment to PR")?;
}

Ok(self_pr)
Expand Down Expand Up @@ -478,7 +516,8 @@ Note: due to a merge conflict, this PR is a hard reset to the starter code, and
.collect::<Vec<_>>(),
)
.send()
.await?;
.await
.with_context(|| format!("Failed to create issue: {}", issue.title))?;
Ok(issue)
}

Expand All @@ -488,17 +527,27 @@ Note: due to a merge conflict, this PR is a hard reset to the starter code, and
.update(issue.number)
.state(IssueState::Closed)
.send()
.await?;
.await
.with_context(|| format!("Failed to close issue: {}", issue.number))?;
Ok(())
}

pub async fn merge_pr(&self, pr: &PullRequest) -> Result<()> {
self.pr_handler().merge(pr.number).send().await?;
self
.pr_handler()
.merge(pr.number)
.send()
.await
.with_context(|| format!("Failed to merge PR: {}", pr.number))?;
Ok(())
}

pub async fn delete(&self) -> Result<()> {
self.repo_handler().delete().await?;
self
.repo_handler()
.delete()
.await
.context("Failed to delete repo")?;
Ok(())
}
}
Expand Down Expand Up @@ -561,7 +610,8 @@ pub fn get_github_token() -> GithubToken {
pub fn init_octocrab(token: &str) -> Result<()> {
let crab_inst = Octocrab::builder()
.personal_token(token.to_string())
.build()?;
.build()
.context("Failed to build Github connector")?;
octocrab::initialise(crab_inst);
Ok(())
}
Loading

0 comments on commit e101a6e

Please sign in to comment.