Jelajahi Sumber

Properly report file exist (check) errors

timvisee 7 tahun lalu
induk
melakukan
1dbb1ee40b
3 mengubah file dengan 67 tambahan dan 27 penghapusan
  1. 54 7
      cli/src/action/download.rs
  2. 1 2
      cli/src/action/info.rs
  3. 12 18
      cli/src/error.rs

+ 54 - 7
cli/src/action/download.rs

@@ -1,16 +1,21 @@
 use std::sync::{Arc, Mutex};
 
 use clap::ArgMatches;
-use ffsend_api::action::download::Download as ApiDownload;
-use ffsend_api::action::exists::Exists as ApiExists;
-use ffsend_api::file::remote_file::RemoteFile;
+use ffsend_api::action::download::{
+    Download as ApiDownload,
+    Error as DownloadError,
+};
+use ffsend_api::action::exists::{
+    Error as ExistsError,
+    Exists as ApiExists,
+};
+use ffsend_api::file::remote_file::{FileParseError, RemoteFile};
 use ffsend_api::reqwest::Client;
 
 use cmd::matcher::{
     Matcher,
     download::DownloadMatcher,
 };
-use error::ActionError;
 use progress::ProgressBar;
 use util::prompt_password;
 
@@ -29,7 +34,7 @@ impl<'a> Download<'a> {
 
     /// Invoke the download action.
     // TODO: create a trait for this method
-    pub fn invoke(&self) -> Result<(), ActionError> {
+    pub fn invoke(&self) -> Result<(), Error> {
         // Create the command matchers
         let matcher_download = DownloadMatcher::with(self.cmd_matches).unwrap();
 
@@ -47,9 +52,13 @@ impl<'a> Download<'a> {
         let target = matcher_download.output();
         let mut password = matcher_download.password();
 
+        // Check whether the file exists
+        let exists = ApiExists::new(&file).invoke(&client)?;
+        if !exists.exists() {
+            return Err(Error::Expired);
+        }
+
         // Check whether the file requires a password
-        // TODO: do not unwrap
-        let exists = ApiExists::new(&file).invoke(&client).unwrap();
         if exists.has_password() != password.is_some() {
             if exists.has_password() {
                 println!("This file is protected with a password.");
@@ -77,3 +86,41 @@ impl<'a> Download<'a> {
         Ok(())
     }
 }
+
+#[derive(Debug, Fail)]
+pub enum Error {
+    /// Failed to parse a share URL, it was invalid.
+    /// This error is not related to a specific action.
+    #[fail(display = "Invalid share URL")]
+    InvalidUrl(#[cause] FileParseError),
+
+    /// An error occurred while checking if the file exists.
+    #[fail(display = "Failed to check whether the file exists")]
+    Exists(#[cause] ExistsError),
+
+    /// An error occurred while downloading the file.
+    #[fail(display = "")]
+    Download(#[cause] DownloadError),
+
+    /// The given Send file has expired, or did never exist in the first place.
+    #[fail(display = "The file has expired or did never exist")]
+    Expired,
+}
+
+impl From<FileParseError> for Error {
+    fn from(err: FileParseError) -> Error {
+        Error::InvalidUrl(err)
+    }
+}
+
+impl From<ExistsError> for Error {
+    fn from(err: ExistsError) -> Error {
+        Error::Exists(err)
+    }
+}
+
+impl From<DownloadError> for Error {
+    fn from(err: DownloadError) -> Error {
+        Error::Download(err)
+    }
+}

+ 1 - 2
cli/src/action/info.rs

@@ -53,8 +53,7 @@ impl<'a> Info<'a> {
         // TODO: show an informative error if the owner token isn't set
 
         // Check whether the file exists
-        // TODO: do not unwrap
-        let exists = ApiExists::new(&file).invoke(&client).unwrap();
+        let exists = ApiExists::new(&file).invoke(&client)?;
         if !exists.exists() {
             return Err(Error::Expired);
         }

+ 12 - 18
cli/src/error.rs

@@ -1,12 +1,12 @@
 use ffsend_api::action::delete::Error as DeleteError;
-use ffsend_api::action::download::Error as DownloadError;
 use ffsend_api::action::exists::Error as ExistsError;
 use ffsend_api::action::params::Error as ParamsError;
 use ffsend_api::action::password::Error as PasswordError;
 use ffsend_api::action::upload::Error as UploadError;
 use ffsend_api::file::remote_file::FileParseError;
 
-use action::info::Error as InfoError;
+use action::download::Error as CliDownloadError;
+use action::info::Error as CliInfoError;
 
 #[derive(Fail, Debug)]
 pub enum Error {
@@ -15,8 +15,14 @@ pub enum Error {
     Action(#[cause] ActionError),
 }
 
-impl From<InfoError> for Error {
-    fn from(err: InfoError) -> Error {
+impl From<CliDownloadError> for Error {
+    fn from(err: CliDownloadError) -> Error {
+        Error::Action(ActionError::Download(err))
+    }
+}
+
+impl From<CliInfoError> for Error {
+    fn from(err: CliInfoError) -> Error {
         Error::Action(ActionError::Info(err))
     }
 }
@@ -35,7 +41,7 @@ pub enum ActionError {
 
     /// An error occurred while invoking the download action.
     #[fail(display = "Failed to download the requested file")]
-    Download(#[cause] DownloadError),
+    Download(#[cause] CliDownloadError),
 
     /// An error occurred while invoking the exists action.
     #[fail(display = "Failed to check whether the file exists")]
@@ -43,7 +49,7 @@ pub enum ActionError {
 
     /// An error occurred while invoking the info action.
     #[fail(display = "Failed to fetch file info")]
-    Info(#[cause] InfoError),
+    Info(#[cause] CliInfoError),
 
     /// An error occurred while invoking the params action.
     #[fail(display = "Failed to change the parameters")]
@@ -70,24 +76,12 @@ impl From<DeleteError> for ActionError {
     }
 }
 
-impl From<DownloadError> for ActionError {
-    fn from(err: DownloadError) -> ActionError {
-        ActionError::Download(err)
-    }
-}
-
 impl From<ExistsError> for ActionError {
     fn from(err: ExistsError) -> ActionError {
         ActionError::Exists(err)
     }
 }
 
-impl From<InfoError> for ActionError {
-    fn from(err: InfoError) -> ActionError {
-        ActionError::Info(err)
-    }
-}
-
 impl From<ParamsError> for ActionError {
     fn from(err: ParamsError) -> ActionError {
         ActionError::Params(err)