Better error handling in src/handlers.rs

Introduces error codes that are shared between JS/RS

Fixes #526.

7 files changed, 269 insertions, 107 deletions

diff --git a/src/binding.rs b/src/binding.rs
index 405f9d7f5..9789fc4e0 100644
--- a/src/binding.rs
+++ b/src/binding.rs
@@ -11,6 +11,7 @@ pub struct DenoC {
 }
 
 #[repr(C)]
+#[derive(PartialEq)]
 pub struct deno_buf {
   pub alloc_ptr: *mut u8,
   pub alloc_len: usize,
diff --git a/src/deno_dir.rs b/src/deno_dir.rs
index 1a8c929ac..72e75bf5b 100644
--- a/src/deno_dir.rs
+++ b/src/deno_dir.rs
@@ -1,8 +1,8 @@
 // Copyright 2018 the Deno authors. All rights reserved. MIT license.
+use errors::DenoError;
 use fs;
 use sha1;
 use std;
-use std::error::Error;
 use std::fs::File;
 use std::io::Write;
 use std::path::Path;
@@ -97,7 +97,7 @@ impl DenoDir {
     self: &DenoDir,
     module_specifier: &str,
     containing_file: &str,
-  ) -> Result<CodeFetchOutput, Box<Error>> {
+  ) -> Result<CodeFetchOutput, DenoError> {
     let (module_name, filename) =
       self.resolve_module(module_specifier, containing_file)?;
 
diff --git a/src/errors.rs b/src/errors.rs
new file mode 100644
index 000000000..f556ba1c2
--- /dev/null
+++ b/src/errors.rs
@@ -0,0 +1,117 @@
+// Copyright 2018 the Deno authors. All rights reserved. MIT license.
+use msg_generated::deno as msg;
+use std;
+use std::fmt;
+use std::io;
+use url;
+
+pub use self::msg::ErrorKind;
+
+pub type DenoResult<T> = std::result::Result<T, DenoError>;
+
+#[derive(Debug)]
+pub struct DenoError {
+  repr: Repr,
+}
+
+impl DenoError {
+  pub fn kind(&self) -> ErrorKind {
+    match self.repr {
+      // Repr::Simple(kind) => kind,
+      Repr::IoErr(ref err) => {
+        use std::io::ErrorKind::*;
+        match err.kind() {
+          NotFound => ErrorKind::NotFound,
+          PermissionDenied => ErrorKind::PermissionDenied,
+          ConnectionRefused => ErrorKind::ConnectionRefused,
+          ConnectionReset => ErrorKind::ConnectionReset,
+          ConnectionAborted => ErrorKind::ConnectionAborted,
+          NotConnected => ErrorKind::NotConnected,
+          AddrInUse => ErrorKind::AddrInUse,
+          AddrNotAvailable => ErrorKind::AddrNotAvailable,
+          BrokenPipe => ErrorKind::BrokenPipe,
+          AlreadyExists => ErrorKind::AlreadyExists,
+          WouldBlock => ErrorKind::WouldBlock,
+          InvalidInput => ErrorKind::InvalidInput,
+          InvalidData => ErrorKind::InvalidData,
+          TimedOut => ErrorKind::TimedOut,
+          Interrupted => ErrorKind::Interrupted,
+          WriteZero => ErrorKind::WriteZero,
+          Other => ErrorKind::Other,
+          UnexpectedEof => ErrorKind::UnexpectedEof,
+          _ => unreachable!(),
+        }
+      }
+      Repr::UrlErr(ref err) => {
+        use url::ParseError::*;
+        match err {
+          EmptyHost => ErrorKind::EmptyHost,
+          IdnaError => ErrorKind::IdnaError,
+          InvalidPort => ErrorKind::InvalidPort,
+          InvalidIpv4Address => ErrorKind::InvalidIpv4Address,
+          InvalidIpv6Address => ErrorKind::InvalidIpv6Address,
+          InvalidDomainCharacter => ErrorKind::InvalidDomainCharacter,
+          RelativeUrlWithoutBase => ErrorKind::RelativeUrlWithoutBase,
+          RelativeUrlWithCannotBeABaseBase => {
+            ErrorKind::RelativeUrlWithCannotBeABaseBase
+          }
+          SetHostOnCannotBeABaseUrl => ErrorKind::SetHostOnCannotBeABaseUrl,
+          Overflow => ErrorKind::Overflow,
+        }
+      }
+    }
+  }
+}
+
+#[derive(Debug)]
+enum Repr {
+  // Simple(ErrorKind),
+  IoErr(io::Error),
+  UrlErr(url::ParseError),
+}
+
+impl fmt::Display for DenoError {
+  fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+    match self.repr {
+      Repr::IoErr(ref err) => err.fmt(f),
+      Repr::UrlErr(ref err) => err.fmt(f),
+      // Repr::Simple(..) => Ok(()),
+    }
+  }
+}
+
+impl std::error::Error for DenoError {
+  fn description(&self) -> &str {
+    match self.repr {
+      Repr::IoErr(ref err) => err.description(),
+      Repr::UrlErr(ref err) => err.description(),
+      // Repr::Simple(..) => "FIXME",
+    }
+  }
+
+  fn cause(&self) -> Option<&std::error::Error> {
+    match self.repr {
+      Repr::IoErr(ref err) => Some(err),
+      Repr::UrlErr(ref err) => Some(err),
+      // Repr::Simple(..) => None,
+    }
+  }
+}
+
+impl From<io::Error> for DenoError {
+  #[inline]
+  fn from(err: io::Error) -> DenoError {
+    DenoError {
+      repr: Repr::IoErr(err),
+    }
+  }
+}
+
+impl From<url::ParseError> for DenoError {
+  #[inline]
+  fn from(err: url::ParseError) -> DenoError {
+    DenoError {
+      repr: Repr::UrlErr(err),
+    }
+  }
+}
diff --git a/src/flags.rs b/src/flags.rs
index 8e40ba73b..b8cbfbf06 100644
--- a/src/flags.rs
+++ b/src/flags.rs
@@ -31,7 +31,8 @@ pub fn print_usage() {
 -r or --reload     Reload cached remote resources.
 -D or --log-debug  Log debug output.
 --help             Print this message.
---v8-options       Print V8 command line options.");
+--v8-options       Print V8 command line options."
+  );
 }
 
 // Parses flags for deno. This does not do v8_set_flags() - call that separately.
diff --git a/src/handlers.rs b/src/handlers.rs
index b40bbc67b..853fbbc77 100644
--- a/src/handlers.rs
+++ b/src/handlers.rs
@@ -1,7 +1,9 @@
 // Copyright 2018 the Deno authors. All rights reserved. MIT license.
 use binding;
-use binding::{deno_buf, deno_set_response, DenoC};
+use binding::{deno_buf, DenoC};
+use errors::DenoResult;
 use flatbuffers;
+use flatbuffers::FlatBufferBuilder;
 use from_c;
 use fs;
 use futures;
@@ -12,22 +14,26 @@ use hyper::Client;
 use msg_generated::deno as msg;
 use std;
 use std::path::Path;
+use std::time::{Duration, Instant};
 use tokio::prelude::future;
+use tokio::prelude::*;
+use tokio::timer::{Delay, Interval};
+
+type HandlerResult = DenoResult<binding::deno_buf>;
 
 pub extern "C" fn msg_from_js(d: *const DenoC, buf: deno_buf) {
   let bytes = unsafe { std::slice::from_raw_parts(buf.data_ptr, buf.data_len) };
   let base = msg::get_root_as_base(bytes);
+  let mut builder = FlatBufferBuilder::new();
   let msg_type = base.msg_type();
-  match msg_type {
-    msg::Any::Start => {
-      reply_start(d);
-    }
+  let result: HandlerResult = match msg_type {
+    msg::Any::Start => handle_start(d, &mut builder),
     msg::Any::CodeFetch => {
       // TODO base.msg_as_CodeFetch();
       let msg = msg::CodeFetch::init_from_table(base.msg().unwrap());
       let module_specifier = msg.module_specifier().unwrap();
       let containing_file = msg.containing_file().unwrap();
-      handle_code_fetch(d, module_specifier, containing_file);
+      handle_code_fetch(d, &mut builder, module_specifier, containing_file)
     }
     msg::Any::CodeCache => {
       // TODO base.msg_as_CodeCache();
@@ -35,51 +41,79 @@ pub extern "C" fn msg_from_js(d: *const DenoC, buf: deno_buf) {
       let filename = msg.filename().unwrap();
       let source_code = msg.source_code().unwrap();
       let output_code = msg.output_code().unwrap();
-      handle_code_cache(d, filename, source_code, output_code);
+      handle_code_cache(d, &mut builder, filename, source_code, output_code)
     }
     msg::Any::FetchReq => {
       // TODO base.msg_as_FetchReq();
       let msg = msg::FetchReq::init_from_table(base.msg().unwrap());
       let url = msg.url().unwrap();
-      handle_fetch_req(d, msg.id(), url);
+      handle_fetch_req(d, &mut builder, msg.id(), url)
     }
     msg::Any::TimerStart => {
       // TODO base.msg_as_TimerStart();
       let msg = msg::TimerStart::init_from_table(base.msg().unwrap());
-      handle_timer_start(d, msg.id(), msg.interval(), msg.delay());
+      handle_timer_start(d, &mut builder, msg.id(), msg.interval(), msg.delay())
     }
     msg::Any::TimerClear => {
       // TODO base.msg_as_TimerClear();
       let msg = msg::TimerClear::init_from_table(base.msg().unwrap());
-      handle_timer_clear(d, msg.id());
+      handle_timer_clear(d, &mut builder, msg.id())
     }
     msg::Any::Exit => {
       // TODO base.msg_as_Exit();
       let msg = msg::Exit::init_from_table(base.msg().unwrap());
-      std::process::exit(msg.code());
+      std::process::exit(msg.code())
     }
     msg::Any::ReadFileSync => {
       // TODO base.msg_as_ReadFileSync();
       let msg = msg::ReadFileSync::init_from_table(base.msg().unwrap());
       let filename = msg.filename().unwrap();
-      handle_read_file_sync(d, filename);
-    }
-    msg::Any::NONE => {
-      assert!(false, "Got message with msg_type == Any_NONE");
+      handle_read_file_sync(d, &mut builder, filename)
     }
-    _ => {
-      assert!(
-        false,
-        format!("Unhandled message {}", msg::enum_name_any(msg_type))
-      );
+    _ => panic!(format!(
+      "Unhandled message {}",
+      msg::enum_name_any(msg_type)
+    )),
+  };
+
+  // No matter whether we got an Err or Ok, we want a serialized message to
+  // send back. So transform the DenoError into a deno_buf.
+  let buf = match result {
+    Err(ref err) => {
+      let errmsg_offset = builder.create_string(&format!("{}", err));
+      create_msg(
+        &mut builder,
+        &msg::BaseArgs {
+          error: Some(errmsg_offset),
+          error_kind: err.kind(),
+          ..Default::default()
+        },
+      )
     }
+    Ok(buf) => buf,
+  };
+
+  // Set the synchronous response, the value returned from deno.send().
+  // null_buf is a special case that indicates success.
+  if buf != null_buf() {
+    unsafe { binding::deno_set_response(d, buf) }
   }
 }
 
-fn reply_start(d: *const DenoC) {
-  let deno = from_c(d);
+fn null_buf() -> deno_buf {
+  deno_buf {
+    alloc_ptr: 0 as *mut u8,
+    alloc_len: 0,
+    data_ptr: 0 as *mut u8,
+    data_len: 0,
+  }
+}
 
-  let mut builder = flatbuffers::FlatBufferBuilder::new();
+fn handle_start(
+  d: *const DenoC,
+  builder: &mut FlatBufferBuilder,
+) -> HandlerResult {
+  let deno = from_c(d);
 
   let argv = deno.argv.iter().map(|s| s.as_str()).collect::<Vec<_>>();
   let argv_off = builder.create_vector_of_strings(argv.as_slice());
@@ -88,7 +122,7 @@ fn reply_start(d: *const DenoC) {
   let cwd_off = builder.create_string(cwd_path.to_str().unwrap());
 
   let msg = msg::StartRes::create(
-    &mut builder,
+    builder,
     &msg::StartResArgs {
       cwd: Some(cwd_off),
       argv: Some(argv_off),
@@ -97,31 +131,18 @@ fn reply_start(d: *const DenoC) {
     },
   );
 
-  set_response_base(
-    d,
-    &mut builder,
+  Ok(create_msg(
+    builder,
     &msg::BaseArgs {
       msg: Some(flatbuffers::Offset::new(msg.value())),
       msg_type: msg::Any::StartRes,
       ..Default::default()
     },
-  )
-}
-
-// TODO(ry) Use Deno instead of DenoC as first arg.
-fn reply_error(d: *const DenoC, cmd_id: u32, msg: &String) {
-  let mut builder = flatbuffers::FlatBufferBuilder::new();
-  // println!("reply_error{}", msg);
-  let args = msg::BaseArgs {
-    cmd_id: cmd_id,
-    error: Some(builder.create_string(msg)),
-    ..Default::default()
-  };
-  set_response_base(d, &mut builder, &args)
+  ))
 }
 
 fn create_msg(
-  builder: &mut flatbuffers::FlatBufferBuilder,
+  builder: &mut FlatBufferBuilder,
   args: &msg::BaseArgs,
 ) -> deno_buf {
   let base = msg::Base::create(builder, &args);
@@ -140,19 +161,9 @@ fn create_msg(
 }
 
 // TODO(ry) Use Deno instead of DenoC as first arg.
-fn set_response_base(
-  d: *const DenoC,
-  builder: &mut flatbuffers::FlatBufferBuilder,
-  args: &msg::BaseArgs,
-) {
-  let buf = create_msg(builder, args);
-  unsafe { deno_set_response(d, buf) }
-}
-
-// TODO(ry) Use Deno instead of DenoC as first arg.
 fn send_base(
   d: *const DenoC,
-  builder: &mut flatbuffers::FlatBufferBuilder,
+  builder: &mut FlatBufferBuilder,
   args: &msg::BaseArgs,
 ) {
   let buf = create_msg(builder, args);
@@ -162,26 +173,16 @@ fn send_base(
 // https://github.com/denoland/deno/blob/golang/os.go#L100-L154
 fn handle_code_fetch(
   d: *const DenoC,
+  builder: &mut FlatBufferBuilder,
   module_specifier: &str,
   containing_file: &str,
-) {
+) -> HandlerResult {
   let deno = from_c(d);
 
   assert!(deno.dir.root.join("gen") == deno.dir.gen, "Sanity check");
 
-  let result = deno
-    .dir
-    .code_fetch(module_specifier, containing_file)
-    .map_err(|err| {
-      let errmsg = format!("{}", err);
-      reply_error(d, 0, &errmsg);
-    });
-  if result.is_err() {
-    return;
-  }
-  let out = result.unwrap();
+  let out = deno.dir.code_fetch(module_specifier, containing_file)?;
   // reply_code_fetch
-  let mut builder = flatbuffers::FlatBufferBuilder::new();
   let mut msg_args = msg::CodeFetchResArgs {
     module_name: Some(builder.create_string(&out.module_name)),
     filename: Some(builder.create_string(&out.filename)),
@@ -194,33 +195,36 @@ fn handle_code_fetch(
     }
     _ => (),
   };
-  let msg = msg::CodeFetchRes::create(&mut builder, &msg_args);
-  let args = msg::BaseArgs {
-    msg: Some(flatbuffers::Offset::new(msg.value())),
-    msg_type: msg::Any::CodeFetchRes,
-    ..Default::default()
-  };
-  set_response_base(d, &mut builder, &args)
+  let msg = msg::CodeFetchRes::create(builder, &msg_args);
+  Ok(create_msg(
+    builder,
+    &msg::BaseArgs {
+      msg: Some(flatbuffers::Offset::new(msg.value())),
+      msg_type: msg::Any::CodeFetchRes,
+      ..Default::default()
+    },
+  ))
 }
 
 // https://github.com/denoland/deno/blob/golang/os.go#L156-L169
 fn handle_code_cache(
   d: *const DenoC,
+  _builder: &mut FlatBufferBuilder,
   filename: &str,
   source_code: &str,
   output_code: &str,
-) {
+) -> HandlerResult {
   let deno = from_c(d);
-  let result = deno.dir.code_cache(filename, source_code, output_code);
-  if result.is_err() {
-    let err = result.unwrap_err();
-    let errmsg = format!("{}", err);
-    reply_error(d, 0, &errmsg);
-  }
-  // null response indicates success.
+  deno.dir.code_cache(filename, source_code, output_code)?;
+  Ok(null_buf()) // null response indicates success.
 }
 
-fn handle_fetch_req(d: *const DenoC, id: u32, url: &str) {
+fn handle_fetch_req(
+  d: *const DenoC,
+  _builder: &mut FlatBufferBuilder,
+  id: u32,
+  url: &str,
+) -> HandlerResult {
   let deno = from_c(d);
   let url = url.parse::<hyper::Uri>().unwrap();
   let client = Client::new();
@@ -304,6 +308,7 @@ fn handle_fetch_req(d: *const DenoC, id: u32, url: &str) {
         );
       }),
   );
+  Ok(null_buf()) // null response indicates success.
 }
 
 fn set_timeout<F>(
@@ -360,7 +365,7 @@ where
 
 // TODO(ry) Use Deno instead of DenoC as first arg.
 fn send_timer_ready(d: *const DenoC, timer_id: u32, done: bool) {
-  let mut builder = flatbuffers::FlatBufferBuilder::new();
+  let mut builder = FlatBufferBuilder::new();
   let msg = msg::TimerReady::create(
     &mut builder,
     &msg::TimerReadyArgs {
@@ -381,37 +386,31 @@ fn send_timer_ready(d: *const DenoC, timer_id: u32, done: bool) {
 }
 
 // Prototype https://github.com/denoland/deno/blob/golang/os.go#L171-L184
-fn handle_read_file_sync(d: *const DenoC, filename: &str) {
+fn handle_read_file_sync(
+  _d: *const DenoC,
+  builder: &mut FlatBufferBuilder,
+  filename: &str,
+) -> HandlerResult {
   debug!("handle_read_file_sync {}", filename);
-  let result = fs::read_file_sync(Path::new(filename));
-  if result.is_err() {
-    let err = result.unwrap_err();
-    let errmsg = format!("{}", err);
-    reply_error(d, 0, &errmsg);
-    return;
-  }
-
+  let vec = fs::read_file_sync(Path::new(filename))?;
   // Build the response message. memcpy data into msg.
   // TODO(ry) zero-copy.
-  let mut builder = flatbuffers::FlatBufferBuilder::new();
-  let vec = result.unwrap();
   let data_off = builder.create_byte_vector(vec.as_slice());
   let msg = msg::ReadFileSyncRes::create(
-    &mut builder,
+    builder,
     &msg::ReadFileSyncResArgs {
       data: Some(data_off),
       ..Default::default()
     },
   );
-  set_response_base(
-    d,
-    &mut builder,
+  Ok(create_msg(
+    builder,
     &msg::BaseArgs {
       msg: Some(flatbuffers::Offset::new(msg.value())),
       msg_type: msg::Any::ReadFileSyncRes,
       ..Default::default()
     },
-  );
+  ))
 }
 
 // TODO(ry) Use Deno instead of DenoC as first arg.
@@ -420,16 +419,14 @@ fn remove_timer(d: *const DenoC, timer_id: u32) {
   deno.timers.remove(&timer_id);
 }
 
-use std::time::{Duration, Instant};
-use tokio::prelude::*;
-use tokio::timer::{Delay, Interval};
 // Prototype: https://github.com/ry/deno/blob/golang/timers.go#L25-L39
 fn handle_timer_start(
   d: *const DenoC,
+  _builder: &mut FlatBufferBuilder,
   timer_id: u32,
   interval: bool,
   delay: u32,
-) {
+) -> HandlerResult {
   debug!("handle_timer_start");
   let deno = from_c(d);
 
@@ -455,10 +452,16 @@ fn handle_timer_start(
     deno.timers.insert(timer_id, cancel_delay);
     deno.rt.spawn(delay_task);
   }
+  Ok(null_buf())
 }
 
 // Prototype: https://github.com/ry/deno/blob/golang/timers.go#L40-L43
-fn handle_timer_clear(d: *const DenoC, timer_id: u32) {
+fn handle_timer_clear(
+  d: *const DenoC,
+  _builder: &mut FlatBufferBuilder,
+  timer_id: u32,
+) -> HandlerResult {
   debug!("handle_timer_clear");
   remove_timer(d, timer_id);
+  Ok(null_buf())
 }
diff --git a/src/main.rs b/src/main.rs
index 2d83b8259..3dc905f20 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -13,6 +13,7 @@ extern crate log;
 
 mod binding;
 mod deno_dir;
+mod errors;
 mod flags;
 mod fs;
 pub mod handlers;
diff --git a/src/msg.fbs b/src/msg.fbs
index 89ecfec45..d1629ba69 100644
--- a/src/msg.fbs
+++ b/src/msg.fbs
@@ -1,6 +1,6 @@
 namespace deno;
 
-union Any { 
+union Any {
   Start,
   StartRes,
   CodeFetch,
@@ -17,8 +17,47 @@ union Any {
   WriteFileSync,
 }
 
+enum ErrorKind: byte {
+  NoError = 0,
+
+  // io errors
+
+  NotFound,
+  PermissionDenied,
+  ConnectionRefused,
+  ConnectionReset,
+  ConnectionAborted,
+  NotConnected,
+  AddrInUse,
+  AddrNotAvailable,
+  BrokenPipe,
+  AlreadyExists,
+  WouldBlock,
+  InvalidInput,
+  InvalidData,
+  TimedOut,
+  Interrupted,
+  WriteZero,
+  Other,
+  UnexpectedEof,
+
+  // url errors
+
+  EmptyHost,
+  IdnaError,
+  InvalidPort,
+  InvalidIpv4Address,
+  InvalidIpv6Address,
+  InvalidDomainCharacter,
+  RelativeUrlWithoutBase,
+  RelativeUrlWithCannotBeABaseBase,
+  SetHostOnCannotBeABaseUrl,
+  Overflow,
+}
+
 table Base {
   cmd_id: uint32;
+  error_kind: ErrorKind = NoError;
   error: string;
   msg: Any;
 }