Skip to content

Commit

Permalink
Use Terminate instead of anyhow::Error in more places
Browse files Browse the repository at this point in the history
`Terminate` is more expressive and than `anyhow::Error`, since it explicitly
takes cancellation into account.
  • Loading branch information
jherbel committed Nov 20, 2024
1 parent 0f10f2a commit b2d71d8
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 10 deletions.
7 changes: 5 additions & 2 deletions src/bin/scheduler/logging.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use anyhow::Error;
use camino::Utf8PathBuf;
use flexi_logger::{
Age, Cleanup, Criterion, DeferredNow, FileSpec, FlexiLoggerError, LogSpecification, Logger,
LoggerHandle, Naming, Record,
};
use log::error;
use std::fmt::Debug;

pub const TIMESTAMP_FORMAT: &str = "%Y-%m-%dT%H.%M.%S%.f%z";

Expand Down Expand Up @@ -43,7 +43,10 @@ pub fn init(
.start()
}

pub fn log_and_return_error(error: Error) -> Error {
pub fn log_and_return_error<T>(error: T) -> T
where
T: Debug,
{
error!("{error:?}");
error
}
12 changes: 7 additions & 5 deletions src/bin/scheduler/scheduling/plans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@ use crate::logging::TIMESTAMP_FORMAT;
use robotmk::plans::run_attempts_with_rebot;
use robotmk::results::{AttemptsConfig, PlanExecutionReport};

use anyhow::{Context, Result as AnyhowResult};
use anyhow::Context;
use chrono::Utc;
use log::info;
use robotmk::section::WritePiggybackSection;
use robotmk::termination::{ContextUnrecoverable, Terminate};
use std::fs::create_dir_all;

pub fn run_plan(plan: &Plan) -> AnyhowResult<()> {
pub fn run_plan(plan: &Plan) -> Result<(), Terminate> {
info!(
"Running plan {} ({})",
&plan.id,
Expand All @@ -21,7 +22,7 @@ pub fn run_plan(plan: &Plan) -> AnyhowResult<()> {
plan.host.clone(),
&plan.results_directory_locker,
)
.context("Reporting plan results failed")?;
.context_unrecoverable("Reporting plan results failed")?;
info!("Plan {} finished", &plan.id);

Ok(())
Expand All @@ -48,7 +49,7 @@ fn format_source_for_logging(source: &Source) -> String {
}
}

fn produce_plan_results(plan: &Plan) -> AnyhowResult<PlanExecutionReport> {
fn produce_plan_results(plan: &Plan) -> Result<PlanExecutionReport, Terminate> {
let timestamp = Utc::now();
let output_directory = plan
.working_directory
Expand All @@ -68,7 +69,8 @@ fn produce_plan_results(plan: &Plan) -> AnyhowResult<PlanExecutionReport> {
&plan.cancellation_token,
&output_directory,
)
.context("Received termination signal while running plan")?;
.map_err(|cancelled| cancelled.into())
.context_unrecoverable("Received termination signal while running plan")?;

Ok(PlanExecutionReport {
plan_id: plan.id.clone(),
Expand Down
7 changes: 4 additions & 3 deletions src/bin/scheduler/setup/directories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use robotmk::environment::Environment;
use robotmk::fs::{create_dir_all, remove_dir_all, remove_file};
use robotmk::results::{plan_results_directory, SetupFailure};
use robotmk::session::Session;
use robotmk::termination::Terminate;
use robotmk::termination::{ContextUnrecoverable, Terminate};
use std::collections::HashSet;

pub fn setup(
Expand Down Expand Up @@ -60,10 +60,11 @@ fn setup_managed_directory(managed_directory: &Utf8Path) -> AnyhowResult<()> {
Ok(())
}

fn setup_results_directory(global_config: &GlobalConfig, plans: &[Plan]) -> AnyhowResult<()> {
fn setup_results_directory(global_config: &GlobalConfig, plans: &[Plan]) -> Result<(), Terminate> {
create_dir_all(&global_config.results_directory)?;
create_dir_all(plan_results_directory(&global_config.results_directory))?;
clean_up_results_directory(global_config, plans).context("Failed to clean up results directory")
clean_up_results_directory(global_config, plans)
.context_unrecoverable("Failed to clean up results directory")
}

fn clean_up_results_directory(
Expand Down
19 changes: 19 additions & 0 deletions src/termination.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::collections::HashSet;
use std::fmt::Display;
use std::future::Future;
use std::time::Duration;
use sysinfo::{Pid, Process, ProcessesToUpdate, System};
Expand All @@ -14,6 +15,24 @@ pub enum Terminate {
Unrecoverable(#[from] anyhow::Error),
}

pub trait ContextUnrecoverable<T> {
fn context_unrecoverable<C>(self, context: C) -> Result<T, Terminate>
where
C: Display + Send + Sync + 'static;
}

impl<T> ContextUnrecoverable<T> for Result<T, Terminate> {
fn context_unrecoverable<C>(self, context: C) -> Result<T, Terminate>
where
C: Display + Send + Sync + 'static,
{
self.map_err(|err| match err {
Terminate::Unrecoverable(any) => Terminate::Unrecoverable(any.context(context)),
Terminate::Cancelled => Terminate::Cancelled,
})
}
}

#[derive(Error, Debug, Clone)]
#[error("Terminated")]
pub struct Cancelled;
Expand Down

0 comments on commit b2d71d8

Please sign in to comment.