Skip to content
Closed
Changes from 3 commits
Commits
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
115 changes: 113 additions & 2 deletions src/persist/io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,34 @@ fn save_json_to_path<T: serde::Serialize>(path: &Path, snapshot: &T) -> std::io:
let tmp_path = target.with_extension("json.tmp");
std::fs::write(&tmp_path, &json)?;
if let Err(err) = std::fs::rename(&tmp_path, &target) {
if is_cross_filesystem_rename_error(err.kind()) {
return write_fallback(&target, &tmp_path, &json);
}
let _ = std::fs::remove_file(&tmp_path);
return Err(err);
}
Ok(())
}

// Writes directly to `target` when `rename` cannot be used. The temp file at
// `tmp_path` still holds a complete copy of `json`, so it is only removed
// once the direct write has actually landed; if the direct write fails
// partway through, the temp file remains as a recovery copy.
fn write_fallback(target: &Path, tmp_path: &Path, json: &str) -> std::io::Result<()> {
let result = std::fs::write(target, json);
if result.is_ok() {
let _ = std::fs::remove_file(tmp_path);
}
Comment thread
greptile-apps[bot] marked this conversation as resolved.
result
}

fn is_cross_filesystem_rename_error(kind: std::io::ErrorKind) -> bool {
matches!(
kind,
std::io::ErrorKind::ResourceBusy | std::io::ErrorKind::CrossesDevices
)
}

pub(super) fn save_to_paths(
session_path: &Path,
history_path: &Path,
Expand Down Expand Up @@ -112,10 +134,21 @@ pub fn clear_history() {

pub fn load() -> Option<SessionSnapshot> {
let path = session_path();
if let Some(snapshot) = try_load_snapshot(&path) {
return Some(snapshot);
}
// The write fallback in `write_fallback` can leave `session.json`
// truncated or invalid if the direct write fails partway, and keeps the
// complete snapshot in `session.json.tmp` for that case. Recover from it
// when the main file is missing or unreadable.
try_load_snapshot(&path.with_extension("json.tmp"))
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Outdated
}

fn try_load_snapshot(path: &Path) -> Option<SessionSnapshot> {
if !path.exists() {
return None;
}
let content = match std::fs::read_to_string(&path) {
let content = match std::fs::read_to_string(path) {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Ok(content) => content,
Err(err) => {
warn!(err = %err, "failed to read session file");
Expand Down Expand Up @@ -143,10 +176,17 @@ pub fn load() -> Option<SessionSnapshot> {

pub fn load_history() -> Option<SessionHistorySnapshot> {
let path = session_history_path();
if let Some(snapshot) = try_load_history(&path) {
return Some(snapshot);
}
try_load_history(&path.with_extension("json.tmp"))
}

fn try_load_history(path: &Path) -> Option<SessionHistorySnapshot> {
if !path.exists() {
return None;
}
let content = match std::fs::read_to_string(&path) {
let content = match std::fs::read_to_string(path) {
Ok(content) => content,
Err(err) => {
warn!(err = %err, "failed to read session history file");
Expand Down Expand Up @@ -337,4 +377,75 @@ mod tests {
.is_symlink());
assert!(target.exists());
}

#[test]
fn cross_filesystem_rename_error_detection() {
assert!(is_cross_filesystem_rename_error(
std::io::ErrorKind::ResourceBusy
));
assert!(is_cross_filesystem_rename_error(
std::io::ErrorKind::CrossesDevices
));
assert!(!is_cross_filesystem_rename_error(
std::io::ErrorKind::PermissionDenied
));
}

#[test]
fn save_to_path_propagates_unrelated_rename_errors() {
// Renaming the temp file onto an existing directory fails with an
// unrelated rename error (the exact kind varies by platform). Herdr
// must still report the error and clean up the temp file.
let target = temp_session_path("rename-onto-directory");
std::fs::create_dir_all(&target).unwrap();

let err = save_to_path(&target, &empty_snapshot()).unwrap_err();

assert!(!is_cross_filesystem_rename_error(err.kind()));
assert!(!target.with_extension("json.tmp").exists());
}

#[test]
fn write_fallback_removes_tmp_file_on_success() {
let target = temp_session_path("fallback-success");
std::fs::create_dir_all(target.parent().unwrap()).unwrap();
let tmp_path = target.with_extension("json.tmp");
std::fs::write(&tmp_path, "{}").unwrap();

write_fallback(&target, &tmp_path, "{}").unwrap();

assert!(target.exists());
assert!(!tmp_path.exists());
}

#[test]
fn write_fallback_keeps_tmp_file_as_recovery_copy_on_failure() {
// `target`'s parent directory does not exist, so the direct write
// fails. The temp file must survive so the last known-good snapshot
// is not lost.
let target = temp_session_path("fallback-failure");
let tmp_path = temp_session_path("fallback-failure-tmp");
std::fs::create_dir_all(tmp_path.parent().unwrap()).unwrap();
std::fs::write(&tmp_path, "{}").unwrap();

write_fallback(&target, &tmp_path, "{}").unwrap_err();

assert!(tmp_path.exists());
}

#[test]
fn try_load_snapshot_recovers_from_tmp_file_when_main_file_is_invalid() {
// Mirrors what `write_fallback` can leave behind: a corrupt main
// file next to a complete `.tmp` file.
let path = temp_session_path("recover-from-tmp");
std::fs::create_dir_all(path.parent().unwrap()).unwrap();
std::fs::write(&path, "not valid json").unwrap();
let mut snap = empty_snapshot();
snap.selected = 5;
save_to_path(&path.with_extension("json.tmp"), &snap).unwrap();

assert!(try_load_snapshot(&path).is_none());
let recovered = try_load_snapshot(&path.with_extension("json.tmp")).unwrap();
assert_eq!(recovered.selected, 5);
}
}
Loading