From 8757834e074c96d65c9d3dd96a4baa2eccefff51 Mon Sep 17 00:00:00 2001 From: Lucien Greathouse Date: Tue, 29 Jan 2019 17:29:47 -0800 Subject: [PATCH] Improve error reporting for IO issues --- CHANGELOG.md | 1 + server/src/commands/build.rs | 10 ++- server/src/commands/serve.rs | 7 ++- server/src/commands/upload.rs | 6 +- server/src/imfs.rs | 61 ++++++++++++++----- server/src/live_session.rs | 5 +- .../missing-files/roblox-project.json | 6 ++ 7 files changed, 74 insertions(+), 22 deletions(-) create mode 100644 test-projects/missing-files/roblox-project.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d42e6bb..8d4e05f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [Unreleased] * Added new (empty) diagnostic page served from the server +* Added better error messages for when a file is missing that's referenced by a Rojo project ## [0.5.0 Alpha 2](https://github.com/LPGhatguy/rojo/releases/tag/v0.5.0-alpha.2) (January 28, 2019) * Added support for `.model.json` files, compatible with 0.4.x diff --git a/server/src/commands/build.rs b/server/src/commands/build.rs index 14819b51..e5880aea 100644 --- a/server/src/commands/build.rs +++ b/server/src/commands/build.rs @@ -10,7 +10,7 @@ use failure::Fail; use crate::{ rbx_session::construct_oneoff_tree, project::{Project, ProjectLoadFuzzyError}, - imfs::Imfs, + imfs::{Imfs, FsError}, }; #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -55,14 +55,18 @@ pub enum BuildError { XmlModelEncodeError(rbx_xml::EncodeError), #[fail(display = "Binary model file error")] - BinaryModelEncodeError(rbx_binary::EncodeError) + BinaryModelEncodeError(rbx_binary::EncodeError), + + #[fail(display = "{}", _0)] + FsError(#[fail(cause)] FsError), } impl_from!(BuildError { ProjectLoadFuzzyError => ProjectLoadError, io::Error => IoError, rbx_xml::EncodeError => XmlModelEncodeError, - rbx_binary::EncodeError => BinaryModelEncodeError + rbx_binary::EncodeError => BinaryModelEncodeError, + FsError => FsError, }); pub fn build(options: &BuildOptions) -> Result<(), BuildError> { diff --git a/server/src/commands/serve.rs b/server/src/commands/serve.rs index 69ec0e75..9b2db9dd 100644 --- a/server/src/commands/serve.rs +++ b/server/src/commands/serve.rs @@ -9,6 +9,7 @@ use failure::Fail; use crate::{ project::{Project, ProjectLoadFuzzyError}, web::Server, + imfs::FsError, live_session::LiveSession, }; @@ -24,10 +25,14 @@ pub struct ServeOptions { pub enum ServeError { #[fail(display = "Project load error: {}", _0)] ProjectLoadError(#[fail(cause)] ProjectLoadFuzzyError), + + #[fail(display = "{}", _0)] + FsError(#[fail(cause)] FsError), } impl_from!(ServeError { ProjectLoadFuzzyError => ProjectLoadError, + FsError => FsError, }); pub fn serve(options: &ServeOptions) -> Result<(), ServeError> { @@ -38,7 +43,7 @@ pub fn serve(options: &ServeOptions) -> Result<(), ServeError> { info!("Found project at {}", project.file_location.display()); info!("Using project {:#?}", project); - let live_session = Arc::new(LiveSession::new(Arc::clone(&project)).unwrap()); + let live_session = Arc::new(LiveSession::new(Arc::clone(&project))?); let server = Server::new(Arc::clone(&live_session)); let port = options.port diff --git a/server/src/commands/upload.rs b/server/src/commands/upload.rs index 229eae6c..0498693c 100644 --- a/server/src/commands/upload.rs +++ b/server/src/commands/upload.rs @@ -11,7 +11,7 @@ use reqwest::header::{ACCEPT, USER_AGENT, CONTENT_TYPE, COOKIE}; use crate::{ rbx_session::construct_oneoff_tree, project::{Project, ProjectLoadFuzzyError}, - imfs::Imfs, + imfs::{Imfs, FsError}, }; #[derive(Debug, Fail)] @@ -33,6 +33,9 @@ pub enum UploadError { #[fail(display = "XML model file error")] XmlModelEncodeError(rbx_xml::EncodeError), + + #[fail(display = "{}", _0)] + FsError(#[fail(cause)] FsError), } impl_from!(UploadError { @@ -40,6 +43,7 @@ impl_from!(UploadError { io::Error => IoError, reqwest::Error => HttpError, rbx_xml::EncodeError => XmlModelEncodeError, + FsError => FsError, }); #[derive(Debug)] diff --git a/server/src/imfs.rs b/server/src/imfs.rs index dec13807..4c30a423 100644 --- a/server/src/imfs.rs +++ b/server/src/imfs.rs @@ -1,15 +1,41 @@ use std::{ collections::{HashMap, HashSet}, path::{self, Path, PathBuf}, + fmt, fs, io, }; +use failure::Fail; use serde_derive::{Serialize, Deserialize}; use crate::project::{Project, ProjectNode}; -fn add_sync_points(imfs: &mut Imfs, project_node: &ProjectNode) -> io::Result<()> { +/// A wrapper around io::Error that also attaches the path associated with the +/// error. +#[derive(Debug, Fail)] +pub struct FsError { + #[fail(cause)] + inner: io::Error, + path: PathBuf, +} + +impl FsError { + fn new>(inner: io::Error, path: P) -> FsError { + FsError { + inner, + path: path.into(), + } + } +} + +impl fmt::Display for FsError { + fn fmt(&self, output: &mut fmt::Formatter) -> fmt::Result { + write!(output, "{}: {}", self.path.display(), self.inner) + } +} + +fn add_sync_points(imfs: &mut Imfs, project_node: &ProjectNode) -> Result<(), FsError> { match project_node { ProjectNode::Instance(node) => { for child in node.children.values() { @@ -44,7 +70,7 @@ impl Imfs { } } - pub fn add_roots_from_project(&mut self, project: &Project) -> io::Result<()> { + pub fn add_roots_from_project(&mut self, project: &Project) -> Result<(), FsError> { add_sync_points(self, &project.tree) } @@ -63,7 +89,7 @@ impl Imfs { self.items.get(path) } - pub fn add_root(&mut self, path: &Path) -> io::Result<()> { + pub fn add_root(&mut self, path: &Path) -> Result<(), FsError> { debug_assert!(path.is_absolute()); debug_assert!(!self.is_within_roots(path)); @@ -84,21 +110,21 @@ impl Imfs { } } - pub fn path_created(&mut self, path: &Path) -> io::Result<()> { + pub fn path_created(&mut self, path: &Path) -> Result<(), FsError> { debug_assert!(path.is_absolute()); debug_assert!(self.is_within_roots(path)); self.descend_and_read_from_disk(path) } - pub fn path_updated(&mut self, path: &Path) -> io::Result<()> { + pub fn path_updated(&mut self, path: &Path) -> Result<(), FsError> { debug_assert!(path.is_absolute()); debug_assert!(self.is_within_roots(path)); self.descend_and_read_from_disk(path) } - pub fn path_removed(&mut self, path: &Path) -> io::Result<()> { + pub fn path_removed(&mut self, path: &Path) -> Result<(), FsError> { debug_assert!(path.is_absolute()); debug_assert!(self.is_within_roots(path)); @@ -111,7 +137,7 @@ impl Imfs { Ok(()) } - pub fn path_moved(&mut self, from_path: &Path, to_path: &Path) -> io::Result<()> { + pub fn path_moved(&mut self, from_path: &Path, to_path: &Path) -> Result<(), FsError> { self.path_removed(from_path)?; self.path_created(to_path)?; Ok(()) @@ -161,9 +187,9 @@ impl Imfs { } } - fn descend_and_read_from_disk(&mut self, path: &Path) -> io::Result<()> { + fn descend_and_read_from_disk(&mut self, path: &Path) -> Result<(), FsError> { let root_path = self.get_root_path(path) - .expect("Tried to mkdirp for path that wasn't within roots!"); + .expect("Tried to descent and read for path that wasn't within roots!"); // If this path is a root, we should read the entire thing. if root_path == path { @@ -193,11 +219,13 @@ impl Imfs { Ok(()) } - fn read_from_disk(&mut self, path: &Path) -> io::Result<()> { - let metadata = fs::metadata(path)?; + fn read_from_disk(&mut self, path: &Path) -> Result<(), FsError> { + let metadata = fs::metadata(path) + .map_err(|e| FsError::new(e, path))?; if metadata.is_file() { - let contents = fs::read(path)?; + let contents = fs::read(path) + .map_err(|e| FsError::new(e, path))?; let item = ImfsItem::File(ImfsFile { path: path.to_path_buf(), contents, @@ -218,8 +246,13 @@ impl Imfs { self.items.insert(path.to_path_buf(), item); - for entry in fs::read_dir(path)? { - let entry = entry?; + let dir_children = fs::read_dir(path) + .map_err(|e| FsError::new(e, path))?; + + for entry in dir_children { + let entry = entry + .map_err(|e| FsError::new(e, path))?; + let child_path = entry.path(); self.read_from_disk(&child_path)?; diff --git a/server/src/live_session.rs b/server/src/live_session.rs index 57f18ac3..f229fb4a 100644 --- a/server/src/live_session.rs +++ b/server/src/live_session.rs @@ -1,11 +1,10 @@ use std::{ sync::{Arc, Mutex}, - io, }; use crate::{ fs_watcher::FsWatcher, - imfs::Imfs, + imfs::{Imfs, FsError}, message_queue::MessageQueue, project::Project, rbx_session::RbxSession, @@ -24,7 +23,7 @@ pub struct LiveSession { } impl LiveSession { - pub fn new(project: Arc) -> io::Result { + pub fn new(project: Arc) -> Result { let imfs = { let mut imfs = Imfs::new(); imfs.add_roots_from_project(&project)?; diff --git a/test-projects/missing-files/roblox-project.json b/test-projects/missing-files/roblox-project.json new file mode 100644 index 00000000..69fc0af0 --- /dev/null +++ b/test-projects/missing-files/roblox-project.json @@ -0,0 +1,6 @@ +{ + "name": "missing-files", + "tree": { + "$path": "does-not-exist" + } +} \ No newline at end of file