Handle write errors gracefully (#143)

This commit is contained in:
Ajeet D'Souza 2021-01-30 03:06:18 +05:30 committed by GitHub
parent d89605ffef
commit 1a5d14a825
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 164 additions and 115 deletions

View File

@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
## Unreleased
### Fixed
- Removed unnecessary backtraces on Rust nightly.
- Fixed generated shell code to avoid accidentally using aliased builtins.
- Handle broken pipe errors gracefully when writing to streams.
## [0.5.0] - 2020-10-30
### Added

View File

@ -114,12 +114,18 @@ zoxide for interactive selection. Installation instructions can be found
### Step 3: Add zoxide to your shell
If you currently use `z`, `z.lua`, or `zsh-z`, you may want to first import
your existing database into `zoxide`:
your existing entries into `zoxide`:
```sh
zoxide import /path/to/db
```
Alternatively, for `autojump`:
```sh
zoxide import --from autojump /path/to/db
```
<!-- omit in toc -->
#### bash

View File

@ -1,6 +1,6 @@
use super::Cmd;
use crate::config;
use crate::store::StoreBuilder;
use crate::db::DatabaseFile;
use crate::util;
use anyhow::Result;
@ -40,10 +40,10 @@ impl Cmd for Add {
let data_dir = config::zo_data_dir()?;
let max_age = config::zo_maxage()?;
let mut store = StoreBuilder::new(data_dir);
let mut store = store.build()?;
store.add(path, now);
store.age(max_age);
let mut db = DatabaseFile::new(data_dir);
let mut db = db.open()?;
db.add(path, now);
db.age(max_age);
Ok(())
}

View File

@ -3,13 +3,13 @@ use crate::config;
use crate::import::{Autojump, Import as _, Z};
use crate::util;
use crate::store::StoreBuilder;
use crate::db::DatabaseFile;
use anyhow::{bail, Result};
use clap::{ArgEnum, Clap};
use std::path::PathBuf;
/// Import entries from another database
/// Import entries from another application
#[derive(Clap, Debug)]
pub struct Import {
path: PathBuf,
@ -27,10 +27,10 @@ impl Cmd for Import {
fn run(&self) -> Result<()> {
let data_dir = config::zo_data_dir()?;
let mut store = StoreBuilder::new(data_dir);
let mut store = store.build()?;
if !self.merge && !store.dirs.is_empty() {
bail!("zoxide database is not empty, specify --merge to continue anyway")
let mut db = DatabaseFile::new(data_dir);
let mut db = db.open()?;
if !self.merge && !db.dirs.is_empty() {
bail!("current database is not empty, specify --merge to continue anyway")
}
let resolve_symlinks = config::zo_resolve_symlinks();
@ -39,8 +39,8 @@ impl Cmd for Import {
resolve_symlinks,
now: util::current_time()?,
}
.import(&mut store, &self.path),
From::Z => Z { resolve_symlinks }.import(&mut store, &self.path),
.import(&mut db, &self.path),
From::Z => Z { resolve_symlinks }.import(&mut db, &self.path),
}
}
}

View File

@ -1,5 +1,6 @@
use super::Cmd;
use crate::config;
use crate::error::WriteErrorHandler;
use crate::shell::{self, Hook, Opts};
use anyhow::{Context, Result};
@ -7,6 +8,8 @@ use askama::Template;
use clap::{ArgEnum, Clap};
use once_cell::sync::OnceCell;
use std::io::{self, Write};
/// Generates shell configuration
#[derive(Clap, Debug)]
#[clap(after_help(env_help()))]
@ -54,9 +57,7 @@ impl Cmd for Init {
Shell::Zsh => shell::Zsh(opts).render(),
}
.context("could not render template")?;
println!("{}", source);
Ok(())
writeln!(io::stdout(), "{}", source).handle_err("stdout")
}
}
@ -75,8 +76,8 @@ fn env_help() -> &'static str {
ENV_HELP.get_or_init(|| {
#[cfg(unix)]
const PATH_SPLIT_SEPARATOR: u8 = b':';
#[cfg(any(target_os = "redox", target_os = "windows"))]
const PATH_SPLIT_SEPARATOR: u8 = b'\\';
#[cfg(windows)]
const PATH_SPLIT_SEPARATOR: u8 = b';';
format!(
"\

View File

@ -1,9 +1,10 @@
use super::Cmd;
use crate::config;
use crate::db::{self, DatabaseFile};
use crate::error::WriteErrorHandler;
use crate::fzf::Fzf;
use crate::util;
use crate::store::{self, StoreBuilder};
use anyhow::{Context, Result};
use clap::Clap;
@ -30,19 +31,20 @@ pub struct Query {
impl Cmd for Query {
fn run(&self) -> Result<()> {
let data_dir = config::zo_data_dir()?;
let mut store = StoreBuilder::new(data_dir);
let mut store = store.build()?;
let mut db = DatabaseFile::new(data_dir);
let mut db = db.open()?;
let query = store::Query::new(&self.keywords);
let query = db::Query::new(&self.keywords);
let now = util::current_time()?;
let mut matches = store.iter_matches(&query, now);
let resolve_symlinks = config::zo_resolve_symlinks();
let mut matches = db.iter_matches(&query, now, resolve_symlinks);
if self.interactive {
let mut fzf = Fzf::new()?;
let handle = fzf.stdin();
for dir in matches {
writeln!(handle, "{}", dir.display_score(now)).context("could not write to fzf")?;
writeln!(handle, "{}", dir.display_score(now)).handle_err("fzf")?;
}
let selection = fzf.wait_select()?;
if self.score {
@ -62,15 +64,16 @@ impl Cmd for Query {
} else {
writeln!(handle, "{}", dir.display())
}
.unwrap()
.handle_err("stdout")?;
}
} else {
let dir = matches.next().context("no match found")?;
if self.score {
println!("{}", dir.display_score(now))
writeln!(io::stdout(), "{}", dir.display_score(now))
} else {
println!("{}", dir.display())
writeln!(io::stdout(), "{}", dir.display())
}
.handle_err("stdout")?;
}
Ok(())

View File

@ -1,7 +1,8 @@
use super::Cmd;
use crate::config;
use crate::db::{DatabaseFile, Query};
use crate::error::WriteErrorHandler;
use crate::fzf::Fzf;
use crate::store::{Query, StoreBuilder};
use crate::util;
use anyhow::{bail, Context, Result};
@ -24,8 +25,8 @@ pub struct Remove {
impl Cmd for Remove {
fn run(&self) -> Result<()> {
let data_dir = config::zo_data_dir()?;
let mut store = StoreBuilder::new(data_dir);
let mut store = store.build()?;
let mut db = DatabaseFile::new(data_dir);
let mut db = db.open()?;
let selection;
let path = match &self.interactive {
@ -35,9 +36,9 @@ impl Cmd for Remove {
let mut fzf = Fzf::new()?;
let handle = fzf.stdin();
for dir in store.iter_matches(&query, now) {
writeln!(handle, "{}", dir.display_score(now))
.context("could not write to fzf")?;
let resolve_symlinks = config::zo_resolve_symlinks();
for dir in db.iter_matches(&query, now, resolve_symlinks) {
writeln!(handle, "{}", dir.display_score(now)).handle_err("fzf")?;
}
selection = fzf.wait_select()?;
@ -48,11 +49,11 @@ impl Cmd for Remove {
None => self.path.as_ref().unwrap(),
};
if !store.remove(path) {
if !db.remove(path) {
let path = util::resolve_path(&path)?;
let path = util::path_to_str(&path)?;
if !store.remove(path) {
bail!("path not found in store: {}", &path)
if !db.remove(path) {
bail!("path not found in database: {}", &path)
}
}

View File

@ -1,4 +1,4 @@
use crate::store::Rank;
use crate::db::Rank;
use anyhow::{bail, Context, Result};
use dirs_next as dirs;
@ -15,7 +15,7 @@ pub fn zo_data_dir() -> Result<PathBuf> {
data_dir.push("zoxide");
data_dir
}
None => bail!("could not find database directory, please set _ZO_DATA_DIR manually"),
None => bail!("could not find data directory, please set _ZO_DATA_DIR manually"),
},
};

View File

@ -6,8 +6,8 @@ use serde::{Deserialize, Serialize};
use std::borrow::Cow;
use std::fmt::{self, Display, Formatter};
use std::fs;
use std::ops::{Deref, DerefMut};
use std::path::Path;
#[derive(Debug, Deserialize, Serialize)]
pub struct DirList<'a>(#[serde(borrow)] Vec<Dir<'a>>);
@ -20,9 +20,9 @@ impl DirList<'_> {
}
pub fn from_bytes(bytes: &[u8]) -> Result<DirList> {
// Assume a maximum size for the store. This prevents bincode from throwing strange
// errors when it encounters invalid data.
const MAX_SIZE: u64 = 8 << 20; // 8 MiB
// Assume a maximum size for the database. This prevents bincode from
// throwing strange errors when it encounters invalid data.
const MAX_SIZE: u64 = 32 << 20; // 32 MiB
let deserializer = &mut bincode::options()
.with_fixint_encoding()
.with_limit(MAX_SIZE);
@ -30,7 +30,7 @@ impl DirList<'_> {
// Split bytes into sections.
let version_size = deserializer.serialized_size(&Self::VERSION).unwrap() as _;
if bytes.len() < version_size {
bail!("could not deserialize store: corrupted data");
bail!("could not deserialize database: corrupted data");
}
let (bytes_version, bytes_dirs) = bytes.split_at(version_size);
@ -46,7 +46,7 @@ impl DirList<'_> {
),
}
})()
.context("could not deserialize store")
.context("could not deserialize database")
}
pub fn to_bytes(&self) -> Result<Vec<u8>> {
@ -62,7 +62,7 @@ impl DirList<'_> {
bincode::serialize_into(&mut buffer, &self)?;
Ok(buffer)
})()
.context("could not serialize store")
.context("could not serialize database")
}
}
@ -95,8 +95,14 @@ pub struct Dir<'a> {
}
impl Dir<'_> {
pub fn is_match(&self, query: &Query) -> bool {
query.matches(&self.path) && Path::new(self.path.as_ref()).is_dir()
pub fn is_match(&self, query: &Query, resolve_symlinks: bool) -> bool {
let resolver = if resolve_symlinks {
fs::symlink_metadata
} else {
fs::metadata
};
let path = self.path.as_ref();
query.matches(path) && resolver(path).map(|m| m.is_dir()).unwrap_or(false)
}
pub fn score(&self, now: Epoch) -> Rank {

View File

@ -14,13 +14,13 @@ use std::fs;
use std::io::{self, Write};
use std::path::{Path, PathBuf};
pub struct Store<'a> {
pub struct Database<'a> {
pub dirs: DirList<'a>,
pub modified: bool,
data_dir: &'a Path,
}
impl<'a> Store<'a> {
impl<'a> Database<'a> {
pub fn save(&mut self) -> Result<()> {
if !self.modified {
return Ok(());
@ -29,7 +29,7 @@ impl<'a> Store<'a> {
let buffer = self.dirs.to_bytes()?;
let mut file = NamedTempFile::new_in(&self.data_dir).with_context(|| {
format!(
"could not create temporary store in: {}",
"could not create temporary database in: {}",
self.data_dir.display()
)
})?;
@ -40,14 +40,14 @@ impl<'a> Store<'a> {
let _ = file.as_file().set_len(buffer.len() as _);
file.write_all(&buffer).with_context(|| {
format!(
"could not write to temporary store: {}",
"could not write to temporary database: {}",
file.path().display()
)
})?;
let path = store_path(&self.data_dir);
let path = db_path(&self.data_dir);
persist(file, &path)
.with_context(|| format!("could not replace store: {}", path.display()))?;
.with_context(|| format!("could not replace database: {}", path.display()))?;
self.modified = false;
Ok(())
@ -76,10 +76,13 @@ impl<'a> Store<'a> {
&'b mut self,
query: &'b Query,
now: Epoch,
resolve_symlinks: bool,
) -> impl DoubleEndedIterator<Item = &'b Dir> {
self.dirs
.sort_unstable_by_key(|dir| Reverse(OrderedFloat(dir.score(now))));
self.dirs.iter().filter(move |dir| dir.is_match(&query))
self.dirs
.iter()
.filter(move |dir| dir.is_match(&query, resolve_symlinks))
}
pub fn remove<S: AsRef<str>>(&mut self, path: S) -> bool {
@ -114,12 +117,12 @@ impl<'a> Store<'a> {
}
}
impl Drop for Store<'_> {
impl Drop for Database<'_> {
fn drop(&mut self) {
// Since the error can't be properly handled here,
// pretty-print it instead.
if let Err(e) = self.save() {
println!("Error: {}", e)
let _ = writeln!(io::stderr(), "zoxide: {:?}", e);
}
}
}
@ -159,29 +162,31 @@ fn persist<P: AsRef<Path>>(file: NamedTempFile, path: P) -> Result<(), PersistEr
Ok(())
}
pub struct StoreBuilder {
pub struct DatabaseFile {
data_dir: PathBuf,
buffer: Vec<u8>,
}
impl StoreBuilder {
pub fn new<P: Into<PathBuf>>(data_dir: P) -> StoreBuilder {
StoreBuilder {
impl DatabaseFile {
pub fn new<P: Into<PathBuf>>(data_dir: P) -> DatabaseFile {
DatabaseFile {
data_dir: data_dir.into(),
buffer: Vec::new(),
}
}
pub fn build(&mut self) -> Result<Store> {
// Read the entire store to memory. For smaller files, this is faster
// than mmap / streaming, and allows for zero-copy deserialization.
let path = store_path(&self.data_dir);
pub fn open(&mut self) -> Result<Database> {
// Read the entire database to memory. For smaller files, this is
// faster than mmap / streaming, and allows for zero-copy
// deserialization.
let path = db_path(&self.data_dir);
match fs::read(&path) {
Ok(buffer) => {
self.buffer = buffer;
let dirs = DirList::from_bytes(&self.buffer)
.with_context(|| format!("could not deserialize store: {}", path.display()))?;
Ok(Store {
let dirs = DirList::from_bytes(&self.buffer).with_context(|| {
format!("could not deserialize database: {}", path.display())
})?;
Ok(Database {
dirs,
modified: false,
data_dir: &self.data_dir,
@ -189,7 +194,7 @@ impl StoreBuilder {
}
Err(e) if e.kind() == io::ErrorKind::NotFound => {
// Create data directory, but don't create any file yet.
// The file will be created later by [`Store::save`]
// The file will be created later by [`Database::save`]
// if any data is modified.
fs::create_dir_all(&self.data_dir).with_context(|| {
format!(
@ -197,22 +202,22 @@ impl StoreBuilder {
self.data_dir.display()
)
})?;
Ok(Store {
Ok(Database {
dirs: DirList::new(),
modified: false,
data_dir: &self.data_dir,
})
}
Err(e) => {
Err(e).with_context(|| format!("could not read from store: {}", path.display()))
Err(e).with_context(|| format!("could not read from database: {}", path.display()))
}
}
}
}
fn store_path<P: AsRef<Path>>(data_dir: P) -> PathBuf {
const STORE_FILENAME: &str = "db.zo";
data_dir.as_ref().join(STORE_FILENAME)
fn db_path<P: AsRef<Path>>(data_dir: P) -> PathBuf {
const DB_FILENAME: &str = "db.zo";
data_dir.as_ref().join(DB_FILENAME)
}
#[cfg(test)]
@ -230,17 +235,17 @@ mod tests {
let data_dir = tempfile::tempdir().unwrap();
{
let mut store = StoreBuilder::new(data_dir.path());
let mut store = store.build().unwrap();
store.add(path, now);
store.add(path, now);
let mut db = DatabaseFile::new(data_dir.path());
let mut db = db.open().unwrap();
db.add(path, now);
db.add(path, now);
}
{
let mut store = StoreBuilder::new(data_dir.path());
let store = store.build().unwrap();
assert_eq!(store.dirs.len(), 1);
let mut db = DatabaseFile::new(data_dir.path());
let db = db.open().unwrap();
assert_eq!(db.dirs.len(), 1);
let dir = &store.dirs[0];
let dir = &db.dirs[0];
assert_eq!(dir.path, path);
assert_eq!(dir.last_accessed, now);
}
@ -257,20 +262,20 @@ mod tests {
let data_dir = tempfile::tempdir().unwrap();
{
let mut store = StoreBuilder::new(data_dir.path());
let mut store = store.build().unwrap();
store.add(path, now);
let mut db = DatabaseFile::new(data_dir.path());
let mut db = db.open().unwrap();
db.add(path, now);
}
{
let mut store = StoreBuilder::new(data_dir.path());
let mut store = store.build().unwrap();
assert!(store.remove(path));
let mut db = DatabaseFile::new(data_dir.path());
let mut db = db.open().unwrap();
assert!(db.remove(path));
}
{
let mut store = StoreBuilder::new(data_dir.path());
let mut store = store.build().unwrap();
assert!(store.dirs.is_empty());
assert!(!store.remove(path));
let mut db = DatabaseFile::new(data_dir.path());
let mut db = db.open().unwrap();
assert!(db.dirs.is_empty());
assert!(!db.remove(path));
}
}
}

View File

@ -1,5 +1,9 @@
use std::fmt::{self, Display, Formatter};
use anyhow::{bail, Context, Result};
use std::fmt::{self, Display, Formatter};
use std::io;
// Custom error type for early exit.
#[derive(Debug)]
pub struct SilentExit {
pub code: i32,
@ -10,3 +14,16 @@ impl Display for SilentExit {
Ok(())
}
}
pub trait WriteErrorHandler {
fn handle_err(self, device: &str) -> Result<()>;
}
impl WriteErrorHandler for io::Result<()> {
fn handle_err(self, device: &str) -> Result<()> {
match self {
Err(e) if e.kind() == io::ErrorKind::BrokenPipe => bail!(SilentExit { code: 0 }),
result => result.with_context(|| format!("could not write to {}", device)),
}
}
}

View File

@ -1,6 +1,6 @@
use super::Import;
use crate::store::{Dir, Epoch, Store};
use crate::db::{Database, Dir, Epoch};
use anyhow::{Context, Result};
use std::borrow::Cow;
@ -13,7 +13,7 @@ pub struct Autojump {
}
impl Import for Autojump {
fn import<P: AsRef<Path>>(&self, store: &mut Store, path: P) -> Result<()> {
fn import<P: AsRef<Path>>(&self, db: &mut Database, path: P) -> Result<()> {
let path = path.as_ref();
let buffer = fs::read_to_string(path)
.with_context(|| format!("could not open autojump database: {}", path.display()))?;
@ -45,13 +45,13 @@ impl Import for Autojump {
let rank_sum = entries.iter().map(|(_, rank)| rank).sum::<f64>();
for &(path, rank) in entries.iter() {
if store.dirs.iter_mut().find(|dir| dir.path == path).is_none() {
store.dirs.push(Dir {
if db.dirs.iter_mut().find(|dir| dir.path == path).is_none() {
db.dirs.push(Dir {
path: Cow::Owned(path.into()),
rank: rank / rank_sum,
last_accessed: self.now,
});
store.modified = true;
db.modified = true;
}
}

View File

@ -1,7 +1,7 @@
mod autojump;
mod z;
use crate::store::Store;
use crate::db::Database;
use anyhow::Result;
use std::path::Path;
@ -10,5 +10,5 @@ pub use autojump::Autojump;
pub use z::Z;
pub trait Import {
fn import<P: AsRef<Path>>(&self, store: &mut Store, path: P) -> Result<()>;
fn import<P: AsRef<Path>>(&self, db: &mut Database, path: P) -> Result<()>;
}

View File

@ -1,6 +1,6 @@
use super::Import;
use crate::store::{Dir, Store};
use crate::db::{Database, Dir};
use anyhow::{Context, Result};
use std::borrow::Cow;
@ -13,7 +13,7 @@ pub struct Z {
}
impl Import for Z {
fn import<P: AsRef<Path>>(&self, store: &mut Store, path: P) -> Result<()> {
fn import<P: AsRef<Path>>(&self, db: &mut Database, path: P) -> Result<()> {
let file = File::open(path).context("could not open z database")?;
let reader = BufReader::new(file);
@ -41,18 +41,18 @@ impl Import for Z {
.parse()
.with_context(|| format!("invalid epoch: {}", last_accessed))?;
match store.dirs.iter_mut().find(|dir| dir.path == path) {
match db.dirs.iter_mut().find(|dir| dir.path == path) {
Some(dir) => {
dir.rank += rank;
dir.last_accessed = dir.last_accessed.max(last_accessed);
}
None => store.dirs.push(Dir {
None => db.dirs.push(Dir {
path: Cow::Owned(path.into()),
rank,
last_accessed,
}),
}
store.modified = true;
db.modified = true;
Ok(())
})()

View File

@ -1,31 +1,33 @@
mod cmd;
mod config;
mod db;
mod error;
mod fzf;
mod import;
mod shell;
mod store;
mod util;
use crate::cmd::{App, Cmd};
use crate::error::SilentExit;
use anyhow::Result;
use clap::Clap;
use std::env;
use std::io::{self, Write};
use std::process;
pub fn main() -> Result<()> {
pub fn main() {
// Forcibly disable backtraces.
env::remove_var("RUST_LIB_BACKTRACE");
env::remove_var("RUST_BACKTRACE");
App::parse()
.run()
.map_err(|e| match e.downcast::<SilentExit>() {
if let Err(e) = App::parse().run() {
match e.downcast::<SilentExit>() {
Ok(SilentExit { code }) => process::exit(code),
// TODO: change the error prefix to `zoxide:`
Err(e) => e,
})
Err(e) => {
let _ = writeln!(io::stderr(), "zoxide: {:?}", e);
process::exit(1);
}
}
}
}

View File

@ -1,4 +1,4 @@
use crate::store::Epoch;
use crate::db::Epoch;
use anyhow::{bail, Context, Result};