From b1d025a23cfe2c058389ad7f2a33b0a5ea005f53 Mon Sep 17 00:00:00 2001
From: koalp <koalp@alpaga.dev>
Date: Fri, 30 Apr 2021 19:54:58 +0200
Subject: [PATCH] feat: move ArticleLocation to defined errors
Previously, the functions in article_location.rs where returning generic
anyhow::Result.
In order to ease error handling when using the library, it have been
moved to specific errors.
---
.gitea/issue_template/bug_report.md | 4 +-
.gitea/issue_template/design_discussion.md | 2 +-
.gitea/issue_template/feature_request.md | 6 +-
.gitea/issue_template/question.md | 2 +-
.gitea/issue_template/refactor.md | 2 +-
crieur-chatbot/src/chatbot.rs | 8 +--
crieur-chatbot/src/handlers/html.rs | 40 +++++++++++--
crieur-retrieve/src/article_location.rs | 65 ++++++++++++++--------
crieur-retrieve/src/lib.rs | 2 +-
src/bin/crieur-chatbot.rs | 1 +
10 files changed, 90 insertions(+), 42 deletions(-)
diff --git a/.gitea/issue_template/bug_report.md b/.gitea/issue_template/bug_report.md
index c450e3d..1aa97a1 100644
--- a/.gitea/issue_template/bug_report.md
+++ b/.gitea/issue_template/bug_report.md
@@ -1,6 +1,6 @@
---
-name: "Bug report"
-about: "This template is for reporting a bug"
+name: "🐛 Bug report"
+about: "For reporting bugs"
title: ""
labels:
- "type::bug"
diff --git a/.gitea/issue_template/design_discussion.md b/.gitea/issue_template/design_discussion.md
index d5a006a..1d68831 100644
--- a/.gitea/issue_template/design_discussion.md
+++ b/.gitea/issue_template/design_discussion.md
@@ -1,5 +1,5 @@
---
-name: "Design discussion"
+name: "🗣 Design discussion"
about: "For discussion about the design of features in the application, when there are several possibilities for implementation"
title: ""
labels:
diff --git a/.gitea/issue_template/feature_request.md b/.gitea/issue_template/feature_request.md
index 3746dda..3f8a60b 100644
--- a/.gitea/issue_template/feature_request.md
+++ b/.gitea/issue_template/feature_request.md
@@ -1,6 +1,6 @@
---
-name: "Feature request"
-about: "This template is for requesting a new feature"
+name: "💡 Feature request"
+about: "For requesting a new feature, with an implementation plan"
title: ""
labels:
- "type::enhancement"
@@ -11,4 +11,4 @@ labels:
*describe what you would like to be able to do, or what solution you would like*
-*(optional) additional context, comments or implementation propositions*
+*(optional) additional context, comments
diff --git a/.gitea/issue_template/question.md b/.gitea/issue_template/question.md
index 92d384d..be3c3d3 100644
--- a/.gitea/issue_template/question.md
+++ b/.gitea/issue_template/question.md
@@ -1,5 +1,5 @@
---
-name: "Ask a question"
+name: "❓ Ask a question"
about: "If you have a question about the usage of the libraries or the tool"
title: ""
labels:
diff --git a/.gitea/issue_template/refactor.md b/.gitea/issue_template/refactor.md
index 6eb7486..e0155dd 100644
--- a/.gitea/issue_template/refactor.md
+++ b/.gitea/issue_template/refactor.md
@@ -1,5 +1,5 @@
---
-name: "Refactor"
+name: "🚧 Refactor"
about: "For refactoring propositions"
title: ""
labels:
diff --git a/crieur-chatbot/src/chatbot.rs b/crieur-chatbot/src/chatbot.rs
index cad6b1c..4052944 100644
--- a/crieur-chatbot/src/chatbot.rs
+++ b/crieur-chatbot/src/chatbot.rs
@@ -46,18 +46,18 @@ impl Builder {
user: &impl AsRef<str>,
password: &impl AsRef<str>,
) -> &mut Self {
- self.user = String::from(user.as_ref());
- self.password = String::from(password.as_ref());
+ self.user = user.as_ref().into();
+ self.password = password.as_ref().into();
self
}
pub(crate) fn homeserver(&mut self, homeserver: &impl AsRef<str>) -> &mut Self {
- self.homeserver = String::from(homeserver.as_ref());
+ self.homeserver = homeserver.as_ref().into();
self
}
pub(crate) fn room(&mut self, room: &impl AsRef<str>) -> &mut Self {
- self.room = String::from(room.as_ref());
+ self.room = room.as_ref().into();
self
}
}
diff --git a/crieur-chatbot/src/handlers/html.rs b/crieur-chatbot/src/handlers/html.rs
index 5cad197..a8a0f83 100644
--- a/crieur-chatbot/src/handlers/html.rs
+++ b/crieur-chatbot/src/handlers/html.rs
@@ -1,8 +1,7 @@
use std::convert::TryInto;
use std::env;
-use anyhow::Result;
-use log::info;
+use log::{error, info};
use matrix_sdk::{
self, async_trait,
events::{
@@ -13,7 +12,7 @@ use matrix_sdk::{
Client, ClientConfig, EventHandler, SyncSettings,
};
-use crieur_retrieve::{ArticleLocation, Url};
+use crieur_retrieve::{article_location::Error, article_location::Result, ArticleLocation, Url};
pub(crate) struct Html {}
@@ -48,12 +47,43 @@ where
//TODO: replace occurences ok() by async and logging block when async block is stable
let article_html = match article_html(url).await {
Ok(url) => url,
- Err(_) => {
- room.send(text_message("Can't download the file"), None)
+ Err(Error::MalformedUrl) => {
+ room.send(text_message("Error: Given url is malformed"), None)
.await
.ok();
return;
}
+ Err(Error::UnknownNewspaper) => {
+ room.send(
+ text_message("Error: Given url is do not correspond to a known newspaper"),
+ None,
+ )
+ .await
+ .ok();
+ return;
+ }
+ Err(Error::Misconfiguration(key)) => {
+ error!(
+ "Error in configuration : {} key is missing or malformed",
+ &key
+ );
+ room.send(
+ text_message("Error: configuration error, please contact your admin"),
+ None,
+ )
+ .await
+ .ok();
+ return;
+ }
+ Err(_) => {
+ room.send(
+ text_message("Unknown error =/, can't download the file"),
+ None,
+ )
+ .await
+ .ok();
+ return;
+ }
};
room.send_attachment(
diff --git a/crieur-retrieve/src/article_location.rs b/crieur-retrieve/src/article_location.rs
index 1cd6178..d6a177a 100644
--- a/crieur-retrieve/src/article_location.rs
+++ b/crieur-retrieve/src/article_location.rs
@@ -2,17 +2,42 @@ use std::boxed::Box;
use std::convert::TryInto;
use std::env;
-use anyhow::{anyhow, Result};
+use anyhow::anyhow;
use log::info;
use url::{Host, Url};
use crate::newspaper::Newspaper;
use crate::newspapers::mediapart::{self, Mediapart};
+/// Enumerate all errors that can be encountered when using ArticleLocation
+#[derive(thiserror::Error, Debug)]
+pub enum Error {
+ /// The url was not set. Therefore, the article location can't be deduced
+ #[error("No url set")]
+ NoUrl,
+ /// The given URL isn't an accepted Url
+ #[error("Malformed URL")]
+ MalformedUrl,
+ /// The given url doesn't correspond to a newspaper.
+ #[error("The given url doesn't link to a known newspaper")]
+ UnknownNewspaper,
+ /// Error in configuration : used for missing or malformed configuration
+ #[error("Error in configuration (configuration key {0} malformed or missing)")]
+ Misconfiguration(String),
+ /// Other errors
+ #[error(transparent)]
+ Other(#[from] anyhow::Error),
+}
+
type Newspapers = Vec<Box<dyn Newspaper>>;
+pub type Result<T, E = Error> = core::result::Result<T, E>;
fn default_newpapers() -> Result<Newspapers> {
- let mpruiid = env::var("MEDIAPART_COOKIE")?.into();
+ let config_key = "MEDIAPART_COOKIE".to_string();
+ let mpruiid = env::var(&config_key)
+ .map_err(|_| Error::Misconfiguration(config_key))?
+ .into();
+
let mediapart = Mediapart::builder()
.login(mediapart::Login::MPRUUID(mpruiid))
.build()?;
@@ -36,13 +61,12 @@ impl Builder {
/// # Errors
///
/// An error is returned if the could not be converted into an url
- // TODO: move this to a defined error, remove anyhow !
pub fn url<U, E>(mut self, url: U) -> Result<Self>
where
U: TryInto<Url, Error = E> + Send,
E: std::error::Error + Sync + Send + 'static,
{
- let url = url.try_into()?;
+ let url = url.try_into().map_err(|_| Error::MalformedUrl)?;
self.url = Some(url);
Ok(self)
}
@@ -60,18 +84,13 @@ impl Builder {
}
/// Adds several newspapers to the list of accepted newspapers
- //fn newspapers(&mut self, newspapers: Newspapers) -> Result<&mut Self> {
- // let newspapers = match &self.newspapers {
- // Some(current_newspapers) => newspapers
- // .iter()
- // .chain(current_newspapers.iter())
- // .map(|s| *(s.clone()))
- // .collect::<Newspapers>(),
- // None => newspapers.into_iter().collect::<Vec<_>>(),
- // };
- // self.newspapers = Some(newspapers);
- // Ok(self)
- //}
+ pub fn newspapers(mut self, newspapers: Newspapers) -> Self {
+ match &mut self.newspapers {
+ Some(current_newspapers) => current_newspapers.extend(newspapers),
+ None => self.newspapers = Some(newspapers.into_iter().collect::<Vec<_>>()),
+ };
+ self
+ }
/// Builds the ArticleLocation by looking which newspaper
///
@@ -82,19 +101,16 @@ impl Builder {
/// - no newpspaper is given
/// - the url is not set
/// - the given url has no host
- // TODO: move this to a defined error, remove anyhow !
pub fn build(self) -> Result<ArticleLocation> {
- let url = Clone::clone(self.url.as_ref().ok_or(anyhow!(
- "No url set. You can set it with the url() function"
- ))?);
- let host = url.host_str().ok_or(anyhow!("Given url has no host"))?;
- let host = Host::parse(host)?;
+ let url = Clone::clone(self.url.as_ref().ok_or(Error::NoUrl)?);
+ let host = url.host_str().ok_or(Error::MalformedUrl)?;
+ let host = Host::parse(host).map_err(|_| Error::MalformedUrl)?;
let newspaper = self
.newspapers
.unwrap_or(default_newpapers()?)
.into_iter()
.find(|c| c.metadata().hosts.contains(&host))
- .ok_or(anyhow!("Newspaper couldn't be found"))?;
+ .ok_or(Error::UnknownNewspaper)?;
Ok(ArticleLocation { newspaper, url })
}
}
@@ -111,6 +127,7 @@ impl ArticleLocation {
pub async fn retrieve_html(&self) -> Result<String> {
info!("It will download from {}", self.url);
- self.newspaper.retrieve_html(&self.url).await
+ // TODO: modify when retrieve_html returns a specific Error type
+ Ok(self.newspaper.retrieve_html(&self.url).await?)
}
}
diff --git a/crieur-retrieve/src/lib.rs b/crieur-retrieve/src/lib.rs
index 31dc3a8..a9f783d 100644
--- a/crieur-retrieve/src/lib.rs
+++ b/crieur-retrieve/src/lib.rs
@@ -10,7 +10,7 @@ pub mod newspaper;
// TODO: move to another crate
pub mod newspapers;
-mod article_location;
+pub mod article_location;
pub use article_location::ArticleLocation;
mod consts;
diff --git a/src/bin/crieur-chatbot.rs b/src/bin/crieur-chatbot.rs
index d0179be..ab4853e 100644
--- a/src/bin/crieur-chatbot.rs
+++ b/src/bin/crieur-chatbot.rs
@@ -4,6 +4,7 @@ use dotenv::dotenv;
#[tokio::main]
async fn main() -> Result<()> {
+ env_logger::init();
dotenv().ok();
run().await?;
Ok(())