refactor error handling with error_chain

6 files changed, 84 insertions, 47 deletions

diff --git a/src/bin/mt-tool.rs b/src/bin/mt-tool.rs
index c04d5ab..e554636 100644
--- a/src/bin/mt-tool.rs
+++ b/src/bin/mt-tool.rs
@@ -7,6 +7,7 @@ extern crate modelica_parser;
 use modelthing::transpile_scheme::TranspileScheme;
 use modelthing::transpile_js::TranspileJS;
 use modelthing::modelica_model::ModelicaModelExt;
+use modelthing::Result;
 use clap::{App, SubCommand};
 use std::io::Read;
 use std::fs::File;
@@ -35,7 +36,7 @@ fn parse_modelica_files(paths: Vec<String>) {
     }
 }
 
-fn main() {
+fn run() -> Result<()> {
 
     let matches = App::new("mt-tool")
         .version(env!("CARGO_PKG_VERSION"))
@@ -69,18 +70,18 @@ fn main() {
             let dir = Path::new(subm.value_of("DIR").unwrap());
             let indep = subm.values_of_lossy("indep").unwrap();
             let dep = subm.values_of_lossy("dep").unwrap();
-            let me = modelthing::load_model_entry(dir).unwrap();
-            println!("{:?}", me.ast.solve_for(indep, dep));
+            let me = try!(modelthing::load_model_entry(dir));
+            println!("{:?}", try!(me.ast.solve_for(indep, dep)));
         },
         ("transpile_scheme", Some(subm)) => {
             let dir = Path::new(subm.value_of("DIR").unwrap());
-            let me = modelthing::load_model_entry(dir).unwrap();
-            println!("{}", me.ast.repr_scheme().unwrap());
+            let me = try!(modelthing::load_model_entry(dir));
+            println!("{}", try!(me.ast.repr_scheme()));
         },
         ("transpile_js", Some(subm)) => {
             let dir = Path::new(subm.value_of("DIR").unwrap());
-            let me = modelthing::load_model_entry(dir).unwrap();
-            println!("{}", me.ast.repr_js().unwrap());
+            let me = try!(modelthing::load_model_entry(dir));
+            println!("{}", try!(me.ast.repr_js()));
         },
         ("list", Some(subm)) => {
             let dir = Path::new(subm.value_of("DIR").unwrap_or("examples"));
@@ -90,7 +91,7 @@ fn main() {
         },
         ("load", Some(subm)) => {
             let dir = Path::new(subm.value_of("DIR").unwrap());
-            let me = modelthing::load_model_entry(dir).unwrap();
+            let me = try!(modelthing::load_model_entry(dir));
             println!("{:?}", me);
         },
         _ => {
@@ -99,4 +100,23 @@ fn main() {
             ::std::process::exit(-1);
         },
     }
+    Ok(())
+}
+
+fn main() {
+    if let Err(ref e) = run() {
+        println!("error: {}", e);
+
+        for e in e.iter().skip(1) {
+            println!("caused by: {}", e);
+        }
+
+        // The backtrace is not always generated. Try to run this example
+        // with `RUST_BACKTRACE=1`.
+        if let Some(backtrace) = e.backtrace() {
+            println!("backtrace: {:?}", backtrace);
+        }
+
+        ::std::process::exit(1);
+    }
 }
diff --git a/src/bin/mt-webface.rs b/src/bin/mt-webface.rs
index b35deec..4cdc509 100644
--- a/src/bin/mt-webface.rs
+++ b/src/bin/mt-webface.rs
@@ -15,6 +15,11 @@ use getopts::Options;
 use pencil::Pencil;
 use pencil::{Request, PencilResult, Response, HTTPError, PencilError};
 
+/*
+This command doesn't use error_chain (or raise errors in general) because the
+web framework handles panics as 500 errors automatically.
+*/
+
 
 fn home(r: &mut Request) -> PencilResult {
     let context: BTreeMap<String,String> = BTreeMap::new();
diff --git a/src/lib.rs b/src/lib.rs
index f93f7fa..24b1ee7 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -2,6 +2,9 @@
 #[macro_use]
 extern crate log;
 
+#[macro_use]
+extern crate error_chain;
+
 extern crate toml;
 extern crate modelica_parser;
 
@@ -14,6 +17,12 @@ use std::fs;
 use std::io::Read;
 use std::fs::File;
 
+mod errors {
+    // Create the Error, ErrorKind, ResultExt, and Result types
+    error_chain! { }
+}
+pub use errors::*;
+
 #[derive(Debug, PartialEq)]
 pub enum ModelVarType {
     Independent,
@@ -39,7 +48,7 @@ pub struct ModelEntry {
     pub markdown: String,
 }
 
-pub fn load_model_entry(p: &Path) -> Result<ModelEntry, String> {
+pub fn load_model_entry(p: &Path) -> Result<ModelEntry> {
     debug!("Attempting to load model from: {:?}", p);
 
     let ast = {
@@ -65,9 +74,13 @@ pub fn load_model_entry(p: &Path) -> Result<ModelEntry, String> {
 }
 
 pub fn search_models(p: &Path) -> Vec<String> {
-    if fs::metadata(p).unwrap().is_dir() {
+    let metadata = match fs::metadata(p) {
+        Err(_) => return vec![],
+        Ok(m) => m,
+    };
+    if metadata.is_dir() {
         fs::read_dir(p)
-            .unwrap()
+            .unwrap()   // exists and is_dir(); could be a race condition though?
             .map(|x| x.unwrap())
             .filter(|x| x.metadata().unwrap().is_dir())
             .filter(|x| x.path().join("model.modelica").exists())
diff --git a/src/modelica_model.rs b/src/modelica_model.rs
index 785b8ae..40b83d6 100644
--- a/src/modelica_model.rs
+++ b/src/modelica_model.rs
@@ -7,9 +7,10 @@ use std::collections::HashSet;
 use std::iter::FromIterator;
 
 use self::modelica_parser::*;
+use errors::Result;
 
 
-/// / Helpers
+/// Helpers
 
 pub trait ModelicaModelExt {
     fn get_constant_vars(&self) -> HashMap<String, Option<Expr>>;
@@ -17,7 +18,7 @@ pub trait ModelicaModelExt {
     fn solve_for(&self,
                  indep_vars: Vec<String>,
                  dep_vars: Vec<String>)
-                 -> Result<ModelicaModel, String>;
+                 -> Result<ModelicaModel>;
 }
 
 impl ModelicaModelExt for ModelicaModel {
@@ -74,7 +75,7 @@ impl ModelicaModelExt for ModelicaModel {
     fn solve_for(&self,
                  indep_vars: Vec<String>,
                  dep_vars: Vec<String>)
-                 -> Result<ModelicaModel, String> {
+                 -> Result<ModelicaModel> {
 
         let indep_vars: HashSet<String> = HashSet::from_iter(indep_vars);
         let dep_vars: HashSet<String> = HashSet::from_iter(dep_vars);
@@ -89,27 +90,27 @@ impl ModelicaModelExt for ModelicaModel {
         passed_vars.extend(dep_vars.clone());
         for var in passed_vars {
             if !all_vars.contains(&var) {
-                return Err(format!("Variable not found in equations: {}", var));
+                return bail!("Variable not found in equations: {}", var);
             }
         }
 
         // check that  V = Q + P + M
         if all_vars.len() != (constants.len() + indep_vars.len() + dep_vars.len()) {
-            return Err(format!("Variable counts don't add up (V={} Q={} P={} M={})",
+            return bail!("Variable counts don't add up (V={} Q={} P={} M={})",
                                all_vars.len(),
                                constants.len(),
                                indep_vars.len(),
-                               dep_vars.len()));
+                               dep_vars.len());
         }
 
         // check that all constants are bound and simple
         for (name, value) in &constants {
             match *value {
-                None => return Err(format!("UnderSpecifiedConstant: {}", name)),
+                None => bail!("UnderSpecifiedConstant: {}", name),
                 Some(Expr::Integer(_)) |
                 Some(Expr::Float(_)) => (), // Ok,
                 Some(_) => {
-                    return Err(format!("NaiveImplementation: can't handle constant: {}", name))
+                    bail!("NaiveImplementation: can't handle constant: {}", name);
                 }
             }
         }
@@ -117,15 +118,14 @@ impl ModelicaModelExt for ModelicaModel {
         // check that there is a depdendent variable in each equation
         for eqn in &self.equations {
             if dep_vars.is_disjoint(&eqn.identifiers()) {
-                return Err("NaiveImplementation/OverConstrained: at least one equation is \
-                            missing a dependent variable"
-                    .to_string());
+                bail!("NaiveImplementation/OverConstrained: at least one equation is \
+                        missing a dependent variable")
             }
         }
 
         // check N >= M
         if self.equations.len() < dep_vars.len() {
-            return Err("UnderConstrained: more dependent variables than equations".to_string());
+            bail!("UnderConstrained: more dependent variables than equations");
         }
 
         println!("Soliving for {:?} in terms of params {:?} and constants {:?}, with {} equations",
@@ -142,7 +142,7 @@ impl ModelicaModelExt for ModelicaModel {
                 .position(|ref x| unsolved_vars.intersection(&x.identifiers()).count() == 1);
             let eqn = match next_i {
                 None => {
-                    return Err("NaiveImplementation (or poor equation selection?)".to_string());
+                    return bail!("NaiveImplementation (or poor equation selection?)");
                 }
                 Some(i) => unsolved_eqns.remove(i),
             };
@@ -191,18 +191,18 @@ pub fn substitute_with(original: &Expr, a: &Expr, b: &Expr) -> Expr {
 }
 
 pub trait SimpleEquationExt {
-    fn rebalance_for(&self, ident: String) -> Result<SimpleEquation, String>;
-    fn simplify_lhs(&self, ident: &str) -> Result<SimpleEquation, String>;
+    fn rebalance_for(&self, ident: String) -> Result<SimpleEquation>;
+    fn simplify_lhs(&self, ident: &str) -> Result<SimpleEquation>;
 }
 
 impl SimpleEquationExt for SimpleEquation {
-    fn rebalance_for(&self, ident: String) -> Result<SimpleEquation, String> {
+    fn rebalance_for(&self, ident: String) -> Result<SimpleEquation> {
         let lvars = self.lhs.identifiers();
         let rvars = self.rhs.identifiers();
 
         let ret = match (lvars.contains(&ident), rvars.contains(&ident)) {
-            (true, true) => Err("SymbolicError: NaiveImplementation".to_string()),
-            (false, false) => Err("SymbolicError: VariableNotFound".to_string()),
+            (true, true) => bail!("SymbolicError: NaiveImplementation"),
+            (false, false) => bail!("SymbolicError: VariableNotFound"),
             (true, false) => self.simplify_lhs(&ident),
             (false, true) => {
                 SimpleEquation {
@@ -215,7 +215,7 @@ impl SimpleEquationExt for SimpleEquation {
         match ret {
             Ok(eqn) => {
                 if eqn.rhs.contains(&ident) {
-                    Err("SymbolicError: NaiveImplementation".to_string())
+                    bail!("SymbolicError: NaiveImplementation")
                 } else {
                     Ok(eqn)
                 }
@@ -224,18 +224,18 @@ impl SimpleEquationExt for SimpleEquation {
         }
     }
 
-    fn simplify_lhs(&self, ident: &str) -> Result<SimpleEquation, String> {
+    fn simplify_lhs(&self, ident: &str) -> Result<SimpleEquation> {
         use modelica_parser::Expr::*;
         use modelica_parser::BinOperator::*;
         match self.lhs {
             Ident(ref s) if s == ident => Ok((*self).clone()),
             Ident(_) | Integer(_) | Float(_) | Boolean(_) | StringLiteral(_) => {
-                Err("SymbolicError: InternalError: expected var on LHS".to_string())
+                bail!("SymbolicError: InternalError: expected var on LHS")
             }
             Der(_) |
             MathUnaryExpr(_, _) |
             Sign(_) |
-            Array(_) => Err("SymbolicError: NaiveImplementation: can't simplify".to_string()),
+            Array(_) => bail!("SymbolicError: NaiveImplementation: can't simplify"),
             // TODO: create a macro for the below...
             BinExpr(Multiply, ref a, ref b) if a.contains(ident) => {
                 SimpleEquation {
@@ -294,8 +294,7 @@ impl SimpleEquationExt for SimpleEquation {
                     .simplify_lhs(&ident)
             }
             BinExpr(_, _, _) => {
-                Err("SymbolicError: NotImplemented BinOperator (or else couldn't find var...)"
-                    .to_string())
+                bail!("SymbolicError: NotImplemented BinOperator (or else couldn't find var...)")
             }
             // in case we add opers: _ => Err("NotImplemented".to_string()),
         }
diff --git a/src/transpile_js.rs b/src/transpile_js.rs
index 5ca16e6..269e900 100644
--- a/src/transpile_js.rs
+++ b/src/transpile_js.rs
@@ -2,14 +2,15 @@
 extern crate modelica_parser;
 
 use self::modelica_parser::*;
+use errors::Result;
 
 pub trait TranspileJS {
-    fn repr_js(&self) -> Result<String, String>;
+    fn repr_js(&self) -> Result<String>;
 }
 
 
 impl TranspileJS for ModelicaModel {
-    fn repr_js(&self) -> Result<String, String> {
+    fn repr_js(&self) -> Result<String> {
         let mut constants = vec![];
         for (c, e) in self.get_constant_vars() {
             if let Some(v) = e {
@@ -23,8 +24,7 @@ impl TranspileJS for ModelicaModel {
                 binds.push(format!("var {} = {};", symb, try!(eq.rhs.repr_js())));
                 outputs.push(symb.to_string());
             } else {
-                return Err("Expected an identifier on LHS (in this partial implementation)"
-                    .to_string());
+                bail!("Expected an identifier on LHS (in this partial implementation)")
             }
         }
         let args: Vec<String> = self.get_free_vars().iter().map(|s| s.clone()).collect();
@@ -42,7 +42,7 @@ impl TranspileJS for ModelicaModel {
 }
 
 impl TranspileJS for Expr {
-    fn repr_js(&self) -> Result<String, String> {
+    fn repr_js(&self) -> Result<String> {
         use modelica_parser::Expr::*;
         match *self {
             Integer(e) => Ok(format!("{}", e)),
@@ -57,7 +57,7 @@ impl TranspileJS for Expr {
             BinExpr(op, ref l, ref r) => {
                 Ok(format!("({} {:?} {})", try!(l.repr_js()), op, try!(r.repr_js())))
             }
-            Array(_) => Err("Array unimplemented".to_string()),
+            Array(_) => bail!("Array unimplemented"),
         }
     }
 }
diff --git a/src/transpile_scheme.rs b/src/transpile_scheme.rs
index 8e6d642..362dfdd 100644
--- a/src/transpile_scheme.rs
+++ b/src/transpile_scheme.rs
@@ -2,14 +2,15 @@
 extern crate modelica_parser;
 
 use self::modelica_parser::*;
+use errors::Result;
 
 pub trait TranspileScheme {
-    fn repr_scheme(&self) -> Result<String, String>;
+    fn repr_scheme(&self) -> Result<String>;
 }
 
 
 impl TranspileScheme for ModelicaModel {
-    fn repr_scheme(&self) -> Result<String, String> {
+    fn repr_scheme(&self) -> Result<String> {
         let mut constants = vec![];
         for (c, e) in self.get_constant_vars() {
             if let Some(v) = e {
@@ -23,8 +24,7 @@ impl TranspileScheme for ModelicaModel {
                 binds.push(format!("({} {})", symb, try!(eq.rhs.repr_scheme())));
                 outputs.push(symb.to_string());
             } else {
-                return Err("Expected an identifier on LHS (in this partial implementation)"
-                    .to_string());
+                bail!("Expected an identifier on LHS (in this partial implementation)")
             }
         }
         let args: Vec<String> = self.get_free_vars().iter().map(|s| s.clone()).collect();
@@ -40,7 +40,7 @@ impl TranspileScheme for ModelicaModel {
 }
 
 impl TranspileScheme for Expr {
-    fn repr_scheme(&self) -> Result<String, String> {
+    fn repr_scheme(&self) -> Result<String> {
         use modelica_parser::Expr::*;
         match *self {
             Integer(e) => Ok(format!("{}", e)),
@@ -58,7 +58,7 @@ impl TranspileScheme for Expr {
                            try!(l.repr_scheme()),
                            try!(r.repr_scheme())))
             }
-            Array(_) => Err("Array unimplemented".to_string()),
+            Array(_) => bail!("Array unimplemented"),
         }
     }
 }