Skip to content

fix: log hook and dispatch errors instead of exiting #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
usefulish wants to merge 1 commit into from
Closed

fix: log hook and dispatch errors instead of exiting #15

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
190 changes: 136 additions & 54 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ use std::{
hash_map::{Entry, HashMap},
},
io,
path::Path,
process::Child,
rc::Rc,
sync::{Arc, LazyLock, Mutex, mpsc},
thread,
Expand All @@ -31,7 +33,76 @@ static CONFLICT_MATCHER: LazyLock<globset::GlobMatcher> = LazyLock::new(|| {
.compile_matcher()
});

#[expect(clippy::too_many_lines)]
/// Map from event kind + folder to the hooks to run
type HookMap = HashMap<(config::FolderEvent, Rc<NormalizedPath>), Vec<config::FolderHook>>;

/// Run a hook, logging errors instead of propagating them, so that a failing hook
/// (typo in command path, resource exhaustion...) can not crash the daemon
fn run_hook(
hook: &config::FolderHook,
path: Option<&Path>,
folder: &NormalizedPath,
reaper_tx: &mpsc::Sender<(hook::FolderHookId, Child)>,
running_hooks: &Arc<Mutex<HashSet<hook::FolderHookId>>>,
) {
if let Err(err) = hook::run(hook, path, folder, reaper_tx, running_hooks) {
log::error!(
"Failed to run hook command {command:?}: {err:#}",
command = hook.command
);
}
}

/// Dispatch a single event to its matching hooks, individual hook run errors are
/// logged and do not interrupt dispatching
fn dispatch_event(
event: &syncthing::Event,
hooks_map: &HookMap,
reaper_tx: &mpsc::Sender<(hook::FolderHookId, Child)>,
running_hooks: &Arc<Mutex<HashSet<hook::FolderHookId>>>,
) -> anyhow::Result<()> {
match event {
syncthing::Event::FileDownSyncDone { path, folder } => {
let folder: Rc<NormalizedPath> = Rc::new(folder.as_path().try_into()?);
for hook in hooks_map
.get(&(config::FolderEvent::FileDownSyncDone, Rc::clone(&folder)))
.unwrap_or(&vec![])
{
if hook.filter.as_ref().is_none_or(|g| g.is_match(path)) {
run_hook(hook, Some(path), &folder, reaper_tx, running_hooks);
}
}
for hook in hooks_map
.get(&(config::FolderEvent::RemoteFileConflict, Rc::clone(&folder)))
.unwrap_or(&vec![])
{
if CONFLICT_MATCHER.is_match(path) {
run_hook(hook, Some(path), &folder, reaper_tx, running_hooks);
}
}
}
syncthing::Event::FolderDownSyncDone { folder } => {
let folder: Rc<NormalizedPath> = Rc::new(folder.as_path().try_into()?);
for hook in hooks_map
.get(&(config::FolderEvent::FolderDownSyncDone, Rc::clone(&folder)))
.unwrap_or(&vec![])
{
run_hook(hook, None, &folder, reaper_tx, running_hooks);
}
}
syncthing::Event::FileConflict { path, folder } => {
let folder: Rc<NormalizedPath> = Rc::new(folder.as_path().try_into()?);
for hook in hooks_map
.get(&(config::FolderEvent::FileConflict, Rc::clone(&folder)))
.unwrap_or(&vec![])
{
run_hook(hook, Some(path), &folder, reaper_tx, running_hooks);
}
}
}
Ok(())
}

fn main() -> anyhow::Result<()> {
// Init logger
simple_logger::SimpleLogger::new()
Expand All @@ -43,8 +114,7 @@ fn main() -> anyhow::Result<()> {
let (cfg, hooks) = config::parse().context("Failed to read local config")?;

// Build hook map for fast matching
let mut hooks_map: HashMap<(config::FolderEvent, Rc<NormalizedPath>), Vec<config::FolderHook>> =
HashMap::new();
let mut hooks_map: HookMap = HashMap::new();
for hook in &hooks.hooks {
match hooks_map.entry((hook.event.clone(), Rc::new(hook.folder.clone()))) {
Entry::Occupied(mut e) => {
Expand Down Expand Up @@ -102,57 +172,11 @@ fn main() -> anyhow::Result<()> {
};
log::info!("New event: {event:?}");

// Dispatch event
match event {
syncthing::Event::FileDownSyncDone { path, folder } => {
let folder: Rc<NormalizedPath> = Rc::new(folder.as_path().try_into()?);
for hook in hooks_map
.get(&(config::FolderEvent::FileDownSyncDone, Rc::clone(&folder)))
.unwrap_or(&vec![])
{
if hook.filter.as_ref().is_none_or(|g| g.is_match(path)) {
hook::run(
hook,
Some(path),
&folder,
&reaper_tx,
&running_hooks,
)?;
}
}
for hook in hooks_map
.get(&(config::FolderEvent::RemoteFileConflict, Rc::clone(&folder)))
.unwrap_or(&vec![])
{
if CONFLICT_MATCHER.is_match(path) {
hook::run(
hook,
Some(path),
&folder,
&reaper_tx,
&running_hooks,
)?;
}
}
}
syncthing::Event::FolderDownSyncDone { folder } => {
let folder: Rc<NormalizedPath> = Rc::new(folder.as_path().try_into()?);
for hook in hooks_map
.get(&(config::FolderEvent::FolderDownSyncDone, Rc::clone(&folder)))
.unwrap_or(&vec![])
{
hook::run(hook, None, &folder, &reaper_tx, &running_hooks)?;
}
}
syncthing::Event::FileConflict { path, folder } => {
let folder: Rc<NormalizedPath> = Rc::new(folder.as_path().try_into()?);
for hook in hooks_map
.get(&(config::FolderEvent::FileConflict, Rc::clone(&folder)))
.unwrap_or(&vec![])
{
hook::run(hook, Some(path), &folder, &reaper_tx, &running_hooks)?;
}
}
// Dispatch event, errors (unresolvable folder path, failing hook...)
// are logged and do not stop the event loop
if let Err(err) = dispatch_event(event, &hooks_map, &reaper_tx, &running_hooks)
{
log::error!("Failed to dispatch event {event:?}: {err:#}");
}
}
cursor = events.cursor();
Expand All @@ -177,6 +201,8 @@ fn main() -> anyhow::Result<()> {

#[cfg(test)]
mod tests {
use std::path::PathBuf;

use super::*;

/// Conflict file names as created by Syncthing must match, at any folder depth
Expand All @@ -187,4 +213,60 @@ mod tests {
assert!(!CONFLICT_MATCHER.is_match("doc.txt"));
assert!(!CONFLICT_MATCHER.is_match("sync-conflict.txt"));
}

/// Hook running `command` for `folder` on file down sync done
fn test_hook(folder: &NormalizedPath, command: &[&str]) -> config::FolderHook {
config::FolderHook {
folder: folder.clone(),
event: config::FolderEvent::FileDownSyncDone,
filter: None,
command: command.iter().map(|a| (*a).to_owned()).collect(),
allow_concurrent: None,
}
}

/// A hook that fails to run must not interrupt dispatching: following hooks still run
#[test]
fn failing_hook_does_not_interrupt_dispatch() {
let dir = tempfile::tempdir().unwrap();
let folder: NormalizedPath = dir.path().try_into().unwrap();
let marker = dir.path().join("marker");
let hooks = vec![
test_hook(&folder, &["/nonexistent/hook/command"]),
test_hook(&folder, &["touch", marker.to_str().unwrap()]),
];
let mut hooks_map = HookMap::new();
hooks_map.insert(
(config::FolderEvent::FileDownSyncDone, Rc::new(folder)),
hooks,
);
let running_hooks = Arc::new(Mutex::new(HashSet::new()));
let (reaper_tx, reaper_rx) = mpsc::channel();
let event = syncthing::Event::FileDownSyncDone {
path: PathBuf::from("file.txt"),
folder: dir.path().to_owned(),
};

dispatch_event(&event, &hooks_map, &reaper_tx, &running_hooks).unwrap();

// Only the second hook spawned a process
let (_hook_id, mut child) = reaper_rx.try_recv().unwrap();
assert!(child.wait().unwrap().success());
assert!(reaper_rx.try_recv().is_err());
assert!(marker.is_file());
}

/// An event folder that does not resolve locally is an error, not an exit or panic
#[test]
fn unresolvable_folder_path_is_an_error() {
let hooks_map = HookMap::new();
let running_hooks = Arc::new(Mutex::new(HashSet::new()));
let (reaper_tx, _reaper_rx) = mpsc::channel();
let event = syncthing::Event::FileDownSyncDone {
path: PathBuf::from("file.txt"),
folder: PathBuf::from("/nonexistent/stfed/folder"),
};

assert!(dispatch_event(&event, &hooks_map, &reaper_tx, &running_hooks).is_err());
}
}
Loading