Skip to content

Commit

Permalink
Handle cache error (#63)
Browse files Browse the repository at this point in the history
* Improve error types

* Handle create file error

* Update changelog

* Fix clippy warnings

* Handle convert host error
  • Loading branch information
quambene authored Nov 30, 2023
1 parent 4ecb0ff commit 5d0067b
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 42 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
- Implement `bogrep -w <pattern>` (match only whole words)
- changed
- Set default for `max_idle_connections_per_host` from 100 to 10
- Fetch: Degrade hard failure to warning message for `BogrepError::CreateFile`
and `BogrepError::ConvertHost`

### v0.5.0

Expand Down
6 changes: 2 additions & 4 deletions src/bookmark_reader/target_reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,10 @@ where
{
fn read(&mut self, target_bookmarks: &mut TargetBookmarks) -> Result<(), BogrepError> {
let mut buf = Vec::new();
self.read_to_end(&mut buf)
.map_err(|err| BogrepError::ReadFile(err.to_string()))?;
self.read_to_end(&mut buf).map_err(BogrepError::ReadFile)?;

// Rewind after reading.
self.rewind()
.map_err(|err| BogrepError::RewindFile(err.to_string()))?;
self.rewind().map_err(BogrepError::RewindFile)?;

let bookmarks = json::deserialize::<BookmarksJson>(&buf)?;

Expand Down
2 changes: 1 addition & 1 deletion src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ impl Caching for Cache {
let mut buf = String::new();
cache_file
.read_to_string(&mut buf)
.map_err(|err| BogrepError::ReadFile(err.to_string()))?;
.map_err(BogrepError::ReadFile)?;
Ok(Some(buf))
} else {
Ok(None)
Expand Down
11 changes: 7 additions & 4 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,16 +76,19 @@ impl Fetch for Client {
if !html.is_empty() {
Ok(html)
} else {
Err(BogrepError::EmptyResponse)
Err(BogrepError::EmptyResponse(bookmark.url.to_owned()))
}
} else {
Err(BogrepError::BinaryResponse)
Err(BogrepError::BinaryResponse(bookmark.url.to_owned()))
}
} else {
Err(BogrepError::BinaryResponse)
Err(BogrepError::BinaryResponse(bookmark.url.to_owned()))
}
} else {
Err(BogrepError::HttpStatus(response.status().to_string()))
Err(BogrepError::HttpStatus {
status: response.status().to_string(),
url: bookmark.url.to_owned(),
})
}
}
}
Expand Down
17 changes: 14 additions & 3 deletions src/cmd/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,22 +152,33 @@ pub async fn fetch_and_add_all(

failed_response += 1;
}
BogrepError::HttpStatus(_) => {
BogrepError::HttpStatus { .. } => {
debug!("{err}");
failed_response += 1;
}
BogrepError::ParseHttpResponse(_) => {
debug!("{err}");
failed_response += 1;
}
BogrepError::BinaryResponse => {
BogrepError::BinaryResponse(_) => {
debug!("{err}");
binary_response += 1;
}
BogrepError::EmptyResponse => {
BogrepError::EmptyResponse(_) => {
debug!("{err}");
empty_response += 1;
}
BogrepError::ConvertHost(_) => {
warn!("{err}");
failed_response += 1;
}
BogrepError::CreateFile { .. } => {
// Write errors are expected if there are "Too many open
// files", so we are issuing a warning instead of returning
// a hard failure.
warn!("{err}");
failed_response += 1;
}
// We are aborting if there is an unexpected error.
err => {
return Err(err);
Expand Down
32 changes: 16 additions & 16 deletions src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,16 @@ pub enum BogrepError {
CreateClient(reqwest::Error),
#[error("Can't fetch website: {0}")]
HttpResponse(reqwest::Error),
#[error("Invalid status code: {0}")]
HttpStatus(String),
#[error("Invalid status code ({url}): {status}")]
HttpStatus { status: String, url: String },
#[error("Can't fetch website: {0}")]
ParseHttpResponse(reqwest::Error),
#[error("Can't fetch binary bookmark")]
BinaryResponse,
#[error("Can't fetch empty bookmark")]
EmptyResponse,
#[error("Can't fetch binary bookmark ({0})")]
BinaryResponse(String),
#[error("Can't fetch empty bookmark ({0})")]
EmptyResponse(String),
#[error("Can't get host for url: {0}")]
ConvertHost(String),
#[error("Can't serialize json: {0}")]
SerializeJson(serde_json::Error),
#[error("Can't deserialize json: {0}")]
Expand All @@ -25,36 +27,34 @@ pub enum BogrepError {
ParseUrl(#[from] ParseError),
#[error("Can't parse url: {0}")]
ConvertHtml(readability::error::Error),
#[error("Can't get host for url: {0}")]
ConvertHost(String),
#[error("Invalid utf8: {0}")]
ConvertUtf8(#[from] FromUtf8Error),
#[error("Can't read from HTML: {0}")]
ReadHtml(io::Error),
#[error("Can't serialize HTML: {0}")]
SerializeHtml(io::Error),
#[error("Can't create file at {path}: {err}")]
CreateFile { path: String, err: String },
CreateFile { path: String, err: io::Error },
#[error("Can't open file at {path}: {err}")]
OpenFile { path: String, err: String },
OpenFile { path: String, err: io::Error },
#[error("Can't remove file at {path}: {err}")]
RemoveFile { path: String, err: String },
RemoveFile { path: String, err: io::Error },
#[error("Can't read file: {0}")]
ReadFile(String),
ReadFile(io::Error),
#[error("Can't write to file at {path}: {err}")]
WriteFile { path: String, err: String },
WriteFile { path: String, err: io::Error },
#[error("Can't append file at {path}: {err}")]
AppendFile { path: String, err: String },
AppendFile { path: String, err: io::Error },
#[error("Can't rename file from {from} to {to}: {err}")]
RenameFile {
from: String,
to: String,
err: String,
err: io::Error,
},
#[error("Can't flush file: {0}")]
FlushFile(io::Error),
#[error("Can't rewind file: {0}")]
RewindFile(String),
RewindFile(io::Error),
#[error("Can't convert header to string: {0}")]
ConvertToStr(#[from] ToStrError),
#[error("Can't remove website ({url}) from cache: {err}")]
Expand Down
28 changes: 14 additions & 14 deletions src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub fn read_file(path: &Path) -> Result<Vec<u8>, BogrepError> {
let mut buffer = Vec::new();
let mut file = open_file(path)?;
file.read_to_end(&mut buffer)
.map_err(|err| BogrepError::ReadFile(err.to_string()))?;
.map_err(BogrepError::ReadFile)?;
Ok(buffer)
}

Expand All @@ -25,7 +25,7 @@ pub fn read_file_to_string(path: &Path) -> Result<String, BogrepError> {
let mut buffer = String::new();
let mut file = open_file(path)?;
file.read_to_string(&mut buffer)
.map_err(|err| BogrepError::ReadFile(err.to_string()))?;
.map_err(BogrepError::ReadFile)?;
Ok(buffer)
}

Expand All @@ -37,7 +37,7 @@ pub fn write_file(path: &Path, content: String) -> Result<(), BogrepError> {
file.write_all(content.as_bytes())
.map_err(|err| BogrepError::WriteFile {
path: path.to_string_lossy().to_string(),
err: err.to_string(),
err,
})?;
file.flush().map_err(BogrepError::FlushFile)?;
Ok(())
Expand All @@ -52,7 +52,7 @@ pub async fn write_file_async(path: &Path, content: &[u8]) -> Result<(), BogrepE
.await
.map_err(|err| BogrepError::WriteFile {
path: path.to_string_lossy().to_string(),
err: err.to_string(),
err,
})?;
file.flush().await.map_err(BogrepError::FlushFile)?;
Ok(())
Expand All @@ -64,7 +64,7 @@ pub fn open_file(path: &Path) -> Result<File, BogrepError> {
debug!("Open file at {}", path.display());
let file = File::open(path).map_err(|err| BogrepError::OpenFile {
path: path.to_string_lossy().to_string(),
err: err.to_string(),
err,
})?;
Ok(file)
}
Expand All @@ -77,7 +77,7 @@ pub fn open_file_in_read_mode(path: &Path) -> Result<File, BogrepError> {
.open(path)
.map_err(|err| BogrepError::OpenFile {
path: path.to_string_lossy().to_string(),
err: err.to_string(),
err,
})?;
Ok(file)
}
Expand All @@ -91,7 +91,7 @@ pub fn open_file_in_read_write_mode(path: &Path) -> Result<File, BogrepError> {
.open(path)
.map_err(|err| BogrepError::OpenFile {
path: path.to_string_lossy().to_string(),
err: err.to_string(),
err,
})?;
Ok(file)
}
Expand All @@ -106,7 +106,7 @@ pub fn open_and_truncate_file(path: &Path) -> Result<File, BogrepError> {
.open(path)
.map_err(|err| BogrepError::OpenFile {
path: path.to_string_lossy().to_string(),
err: err.to_string(),
err,
})?;
Ok(file)
}
Expand All @@ -116,7 +116,7 @@ pub fn create_file(path: &Path) -> Result<File, BogrepError> {
debug!("Create file at {}", path.display());
let file = File::create(path).map_err(|err| BogrepError::CreateFile {
path: path.to_string_lossy().to_string(),
err: err.to_string(),
err,
})?;
Ok(file)
}
Expand All @@ -128,7 +128,7 @@ pub async fn create_file_async(path: &Path) -> Result<tokio::fs::File, BogrepErr
.await
.map_err(|err| BogrepError::CreateFile {
path: path.to_string_lossy().to_string(),
err: err.to_string(),
err,
})?;
Ok(file)
}
Expand All @@ -142,7 +142,7 @@ pub fn append_file(path: &Path) -> Result<File, BogrepError> {
.open(path)
.map_err(|err| BogrepError::AppendFile {
path: path.to_string_lossy().to_string(),
err: err.to_string(),
err,
})?;
Ok(file)
}
Expand All @@ -152,7 +152,7 @@ pub fn remove_file(path: &Path) -> Result<(), BogrepError> {
debug!("Remove file at {}", path.display());
fs::remove_file(path).map_err(|err| BogrepError::RemoveFile {
path: path.to_string_lossy().to_string(),
err: err.to_string(),
err,
})
}

Expand All @@ -163,7 +163,7 @@ pub async fn remove_file_async(path: &Path) -> Result<(), BogrepError> {
.await
.map_err(|err| BogrepError::RemoveFile {
path: path.to_string_lossy().to_string(),
err: err.to_string(),
err,
})
}

Expand All @@ -183,7 +183,7 @@ pub fn close_and_rename(from: (File, &Path), to: (File, &Path)) -> Result<(), Bo
fs::rename(from.1, to.1).map_err(|err| BogrepError::RenameFile {
from: from.1.to_string_lossy().to_string(),
to: to.1.to_string_lossy().to_string(),
err: err.to_string(),
err,
})?;

Ok(())
Expand Down

0 comments on commit 5d0067b

Please sign in to comment.