From 20be37dd8ba04b3a7ddae177923e3d6afe5ebe36 Mon Sep 17 00:00:00 2001 From: Lucien Greathouse Date: Wed, 27 Feb 2019 00:47:02 -0800 Subject: [PATCH] Improve error messages from bad snapshots --- CHANGELOG.md | 1 + server/src/commands/build.rs | 11 +++-- server/src/commands/serve.rs | 6 ++- server/src/commands/upload.rs | 11 +++-- server/src/live_session.rs | 21 +++++++- server/src/rbx_session.rs | 49 +++++++++++++------ .../malformed-stuff/default.project.json | 6 +++ .../malformed-stuff/src/bad-model.model.json | 2 + 8 files changed, 83 insertions(+), 24 deletions(-) create mode 100644 test-projects/malformed-stuff/default.project.json create mode 100644 test-projects/malformed-stuff/src/bad-model.model.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 9de2f88e..b5f4d18f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * Fixed plugin assets flashing in on first load ([#121](https://github.com/LPGhatguy/rojo/issues/121)) * Changed Rojo's HTTP server from Rouille to Hyper, which reduced the release size by around a megabyte. * Added property type inference to projects, which makes specifying services a lot easier ([#130](https://github.com/LPGhatguy/rojo/pull/130)) +* Made error messages from invalid and missing files more user-friendly ## [0.5.0 Alpha 4](https://github.com/LPGhatguy/rojo/releases/tag/v0.5.0-alpha.4) (February 8, 2019) * Added support for nested partitions ([#102](https://github.com/LPGhatguy/rojo/issues/102)) diff --git a/server/src/commands/build.rs b/server/src/commands/build.rs index 2c839e69..16301ed3 100644 --- a/server/src/commands/build.rs +++ b/server/src/commands/build.rs @@ -8,9 +8,10 @@ use log::info; use failure::Fail; use crate::{ - rbx_session::construct_oneoff_tree, - project::{Project, ProjectLoadFuzzyError}, imfs::{Imfs, FsError}, + project::{Project, ProjectLoadFuzzyError}, + rbx_session::construct_oneoff_tree, + rbx_snapshot::SnapshotError, }; #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -59,6 +60,9 @@ pub enum BuildError { #[fail(display = "{}", _0)] FsError(#[fail(cause)] FsError), + + #[fail(display = "{}", _0)] + SnapshotError(#[fail(cause)] SnapshotError), } impl_from!(BuildError { @@ -67,6 +71,7 @@ impl_from!(BuildError { rbx_xml::EncodeError => XmlModelEncodeError, rbx_binary::EncodeError => BinaryModelEncodeError, FsError => FsError, + SnapshotError => SnapshotError, }); pub fn build(options: &BuildOptions) -> Result<(), BuildError> { @@ -86,7 +91,7 @@ pub fn build(options: &BuildOptions) -> Result<(), BuildError> { let mut imfs = Imfs::new(); imfs.add_roots_from_project(&project)?; - let tree = construct_oneoff_tree(&project, &imfs); + let tree = construct_oneoff_tree(&project, &imfs)?; let mut file = File::create(&options.output_file)?; match output_kind { diff --git a/server/src/commands/serve.rs b/server/src/commands/serve.rs index 9e7fac63..896bbc1b 100644 --- a/server/src/commands/serve.rs +++ b/server/src/commands/serve.rs @@ -10,7 +10,7 @@ use crate::{ project::{Project, ProjectLoadFuzzyError}, web::LiveServer, imfs::FsError, - live_session::LiveSession, + live_session::{LiveSession, LiveSessionError}, }; const DEFAULT_PORT: u16 = 34872; @@ -28,11 +28,15 @@ pub enum ServeError { #[fail(display = "{}", _0)] FsError(#[fail(cause)] FsError), + + #[fail(display = "{}", _0)] + LiveSessionError(#[fail(cause)] LiveSessionError), } impl_from!(ServeError { ProjectLoadFuzzyError => ProjectLoadError, FsError => FsError, + LiveSessionError => LiveSessionError, }); pub fn serve(options: &ServeOptions) -> Result<(), ServeError> { diff --git a/server/src/commands/upload.rs b/server/src/commands/upload.rs index fb26e0ab..a142d43e 100644 --- a/server/src/commands/upload.rs +++ b/server/src/commands/upload.rs @@ -9,9 +9,10 @@ use failure::Fail; use reqwest::header::{ACCEPT, USER_AGENT, CONTENT_TYPE, COOKIE}; use crate::{ - rbx_session::construct_oneoff_tree, - project::{Project, ProjectLoadFuzzyError}, imfs::{Imfs, FsError}, + project::{Project, ProjectLoadFuzzyError}, + rbx_session::construct_oneoff_tree, + rbx_snapshot::SnapshotError, }; #[derive(Debug, Fail)] @@ -36,6 +37,9 @@ pub enum UploadError { #[fail(display = "{}", _0)] FsError(#[fail(cause)] FsError), + + #[fail(display = "{}", _0)] + SnapshotError(#[fail(cause)] SnapshotError), } impl_from!(UploadError { @@ -44,6 +48,7 @@ impl_from!(UploadError { reqwest::Error => HttpError, rbx_xml::EncodeError => XmlModelEncodeError, FsError => FsError, + SnapshotError => SnapshotError, }); #[derive(Debug)] @@ -67,7 +72,7 @@ pub fn upload(options: &UploadOptions) -> Result<(), UploadError> { let mut imfs = Imfs::new(); imfs.add_roots_from_project(&project)?; - let tree = construct_oneoff_tree(&project, &imfs); + let tree = construct_oneoff_tree(&project, &imfs)?; let root_id = tree.get_root_id(); let mut contents = Vec::new(); diff --git a/server/src/live_session.rs b/server/src/live_session.rs index f229fb4a..668353e1 100644 --- a/server/src/live_session.rs +++ b/server/src/live_session.rs @@ -2,16 +2,33 @@ use std::{ sync::{Arc, Mutex}, }; +use failure::Fail; + use crate::{ fs_watcher::FsWatcher, imfs::{Imfs, FsError}, message_queue::MessageQueue, project::Project, rbx_session::RbxSession, + rbx_snapshot::SnapshotError, session_id::SessionId, snapshot_reconciler::InstanceChanges, }; +#[derive(Debug, Fail)] +pub enum LiveSessionError { + #[fail(display = "{}", _0)] + Fs(#[fail(cause)] FsError), + + #[fail(display = "{}", _0)] + Snapshot(#[fail(cause)] SnapshotError), +} + +impl_from!(LiveSessionError { + FsError => Fs, + SnapshotError => Snapshot, +}); + /// Contains all of the state for a Rojo live-sync session. pub struct LiveSession { pub project: Arc, @@ -23,7 +40,7 @@ pub struct LiveSession { } impl LiveSession { - pub fn new(project: Arc) -> Result { + pub fn new(project: Arc) -> Result { let imfs = { let mut imfs = Imfs::new(); imfs.add_roots_from_project(&project)?; @@ -36,7 +53,7 @@ impl LiveSession { Arc::clone(&project), Arc::clone(&imfs), Arc::clone(&message_queue), - ))); + )?)); let fs_watcher = FsWatcher::start( Arc::clone(&imfs), diff --git a/server/src/rbx_session.rs b/server/src/rbx_session.rs index 6927cff8..f2bb71d0 100644 --- a/server/src/rbx_session.rs +++ b/server/src/rbx_session.rs @@ -8,7 +8,7 @@ use std::{ use rlua::Lua; use serde_derive::{Serialize, Deserialize}; -use log::{info, trace}; +use log::{info, trace, error}; use rbx_dom_weak::{RbxTree, RbxId}; use crate::{ @@ -17,6 +17,7 @@ use crate::{ imfs::{Imfs, ImfsItem}, path_map::PathMap, rbx_snapshot::{ + SnapshotError, SnapshotContext, SnapshotPluginContext, SnapshotPluginEntry, @@ -31,6 +32,12 @@ const INIT_SCRIPT: &str = "init.lua"; const INIT_SERVER_SCRIPT: &str = "init.server.lua"; const INIT_CLIENT_SCRIPT: &str = "init.client.lua"; +fn show_snapshot_error(path: &Path, error: SnapshotError) { + error!("Rojo couldn't turn one of the project's files into Roblox instances."); + error!("Any changes to the file have been ignored."); + error!("{}", error); +} + /// `source_path` or `project_definition` or both must both be Some. #[derive(Debug, Clone, PartialEq, Default, Serialize, Deserialize)] pub struct MetadataPerInstance { @@ -66,7 +73,7 @@ impl RbxSession { project: Arc, imfs: Arc>, message_queue: Arc>, - ) -> RbxSession { + ) -> Result { let mut instances_per_path = PathMap::new(); let mut metadata_per_instance = HashMap::new(); @@ -104,16 +111,22 @@ impl RbxSession { let tree = { let temp_imfs = imfs.lock().unwrap(); - reify_initial_tree(&project, &context, &temp_imfs, &mut instances_per_path, &mut metadata_per_instance) + reify_initial_tree( + &project, + &context, + &temp_imfs, + &mut instances_per_path, + &mut metadata_per_instance, + )? }; - RbxSession { + Ok(RbxSession { tree, instances_per_path, metadata_per_instance, message_queue, imfs, - } + }) } fn path_created_or_updated(&mut self, path: &Path) { @@ -155,20 +168,24 @@ impl RbxSession { let maybe_snapshot = match &instance_metadata.project_definition { Some((instance_name, project_node)) => { snapshot_project_node(&context, &imfs, &project_node, Cow::Owned(instance_name.clone())) - .unwrap_or_else(|_| panic!("Could not generate instance snapshot for path {}", path_to_snapshot.display())) + // .unwrap_or_else(|_| panic!("Could not generate instance snapshot for path {}", path_to_snapshot.display())) }, None => { snapshot_imfs_path(&context, &imfs, &path_to_snapshot, None) - .unwrap_or_else(|_| panic!("Could not generate instance snapshot for path {}", path_to_snapshot.display())) + // .unwrap_or_else(|_| panic!("Could not generate instance snapshot for path {}", path_to_snapshot.display())) }, }; let snapshot = match maybe_snapshot { - Some(snapshot) => snapshot, - None => { + Ok(Some(snapshot)) => snapshot, + Ok(None) => { trace!("Path resulted in no snapshot being generated."); return; }, + Err(err) => { + show_snapshot_error(&path_to_snapshot, err); + return; + }, }; trace!("Snapshot: {:#?}", snapshot); @@ -243,12 +260,13 @@ impl RbxSession { } } -pub fn construct_oneoff_tree(project: &Project, imfs: &Imfs) -> RbxTree { +pub fn construct_oneoff_tree(project: &Project, imfs: &Imfs) -> Result { let mut instances_per_path = PathMap::new(); let mut metadata_per_instance = HashMap::new(); let context = SnapshotContext { plugin_context: None, }; + reify_initial_tree(project, &context, imfs, &mut instances_per_path, &mut metadata_per_instance) } @@ -258,13 +276,14 @@ fn reify_initial_tree( imfs: &Imfs, instances_per_path: &mut PathMap>, metadata_per_instance: &mut HashMap, -) -> RbxTree { - let snapshot = snapshot_project_tree(&context, imfs, project) - .expect("Could not snapshot project tree") - .expect("Project did not produce any instances"); +) -> Result { + let snapshot = match snapshot_project_tree(&context, imfs, project)? { + Some(snapshot) => snapshot, + None => panic!("Project did not produce any instances"), + }; let mut changes = InstanceChanges::default(); let tree = reify_root(&snapshot, instances_per_path, metadata_per_instance, &mut changes); - tree + Ok(tree) } \ No newline at end of file diff --git a/test-projects/malformed-stuff/default.project.json b/test-projects/malformed-stuff/default.project.json new file mode 100644 index 00000000..710bd36c --- /dev/null +++ b/test-projects/malformed-stuff/default.project.json @@ -0,0 +1,6 @@ +{ + "name": "malformed-stuff", + "tree": { + "$path": "src" + } +} \ No newline at end of file diff --git a/test-projects/malformed-stuff/src/bad-model.model.json b/test-projects/malformed-stuff/src/bad-model.model.json new file mode 100644 index 00000000..b6fc298c --- /dev/null +++ b/test-projects/malformed-stuff/src/bad-model.model.json @@ -0,0 +1,2 @@ +ahhh this isn't a JSON model +bamboozled again \ No newline at end of file