From c6a24134308ee90a39bdea907b525292a227eadf Mon Sep 17 00:00:00 2001 From: "Kevin (Kun) \"Kassimo\" Qian" Date: Fri, 28 Sep 2018 21:21:47 -0700 Subject: [PATCH 01/11] Start try extension attempt --- src/deno_dir.rs | 56 ++++++++++++++++++++++++++++++++++++------------- src/http.rs | 16 +++++++++----- 2 files changed, 53 insertions(+), 19 deletions(-) diff --git a/src/deno_dir.rs b/src/deno_dir.rs index cb769fb86517b1..dfc271d2e7bfd5 100644 --- a/src/deno_dir.rs +++ b/src/deno_dir.rs @@ -133,9 +133,28 @@ impl DenoDir { self: &DenoDir, module_name: &str, filename: &str, - ) -> DenoResult { + ) -> DenoResult { if is_remote(module_name) { - self.fetch_remote_source(module_name, filename) + let try_extension = |ext| { + let module_name = format!("{}{}", module_name, ext); + let filename = format!("{}{}", filename, ext); + let source_code = self.fetch_remote_source(&module_name, &filename)?; + return Ok(CodeFetchOutput { + module_name: module_name.to_string(), + filename: filename.to_string(), + source_code, + maybe_output_code: None, + }); + }; + if module_name.ends_with(".ts") || module_name.ends_with(".js") { + return try_extension(""); + } + println!("Trying {}.ts...", module_name); + if let Ok(v) = try_extension(".ts") { + return Ok(v); + } + println!("Trying {}.js...", module_name); + try_extension(".js") } else if module_name.starts_with(ASSET_PREFIX) { panic!("Asset resolution should be done in JS, not Rust."); } else { @@ -143,8 +162,26 @@ impl DenoDir { module_name, filename, "if a module isn't remote, it should have the same filename" ); - let src = fs::read_to_string(Path::new(filename))?; - Ok(src) + let try_extension = |ext| { + let module_name = format!("{}{}", module_name, ext); + let filename = format!("{}{}", filename, ext); + let source_code = fs::read_to_string(Path::new(&filename))?; + return Ok(CodeFetchOutput { + module_name: module_name.to_string(), + filename: filename.to_string(), + source_code, + maybe_output_code: None, + }); + }; + if module_name.ends_with(".ts") || module_name.ends_with(".js") { + return try_extension(""); + } + println!("Trying {}.ts...", module_name); + if let Ok(v) = try_extension(".ts") { + return Ok(v); + } + println!("Trying {}.js...", module_name); + try_extension(".js") } } @@ -161,16 +198,7 @@ impl DenoDir { let (module_name, filename) = self.resolve_module(module_specifier, containing_file)?; - let result = self - .get_source_code(module_name.as_str(), filename.as_str()) - .and_then(|source_code| { - Ok(CodeFetchOutput { - module_name, - filename, - source_code, - maybe_output_code: None, - }) - }); + let result = self.get_source_code(module_name.as_str(), filename.as_str()); let out = match result { Err(err) => { if err.kind() == ErrorKind::NotFound { diff --git a/src/http.rs b/src/http.rs index 5f78cf232c7a33..cbb1ba9b114d59 100644 --- a/src/http.rs +++ b/src/http.rs @@ -3,13 +3,14 @@ use errors::DenoResult; use tokio_util; -use futures::Future; +// use futures::Future; use futures::Stream; use hyper; use hyper::client::Client; use hyper::client::HttpConnector; use hyper::Uri; use hyper_rustls; +use std::io; type Connector = hyper_rustls::HttpsConnector; @@ -31,10 +32,15 @@ pub fn get_client() -> Client { pub fn fetch_sync_string(module_name: &str) -> DenoResult { let url = module_name.parse::().unwrap(); let client = get_client(); - let future = client - .get(url) - .and_then(|response| response.into_body().concat2()); - let body = tokio_util::block_on(future)?; + let get_future = client.get(url); + let response = tokio_util::block_on(get_future)?; + if !response.status().is_success() { + Err(io::Error::new( + io::ErrorKind::NotFound, + format!("cannot load from '{}'", module_name), + ))?; + } + let body = tokio_util::block_on(response.into_body().concat2())?; Ok(String::from_utf8(body.to_vec()).unwrap()) } From bffb412b23178641e7bdb6e5d66ddcf86d4de8ad Mon Sep 17 00:00:00 2001 From: "Kevin (Kun) \"Kassimo\" Qian" Date: Fri, 28 Sep 2018 21:48:24 -0700 Subject: [PATCH 02/11] Add basic rust-side test --- src/deno_dir.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/deno_dir.rs b/src/deno_dir.rs index dfc271d2e7bfd5..5d96b2910a7b3b 100644 --- a/src/deno_dir.rs +++ b/src/deno_dir.rs @@ -448,6 +448,26 @@ fn test_code_fetch() { //println!("code_fetch_output {:?}", code_fetch_output); } +#[test] +fn test_code_fetch_no_ext() { + let (_temp_dir, deno_dir) = test_setup(); + + let cwd = std::env::current_dir().unwrap(); + let cwd_string = String::from(cwd.to_str().unwrap()) + "/"; + + // Assuming cwd is the deno repo root. + let module_specifier = "./js/main"; + let containing_file = cwd_string.as_str(); + let r = deno_dir.code_fetch(module_specifier, containing_file); + assert!(r.is_ok()); + + // Assuming cwd is the deno repo root. + let module_specifier = "./js/mock_builtin"; + let containing_file = cwd_string.as_str(); + let r = deno_dir.code_fetch(module_specifier, containing_file); + assert!(r.is_ok()); +} + #[test] fn test_src_file_to_url_1() { let (_temp_dir, deno_dir) = test_setup(); From 699822bd03889f8cdaf709f7c08651c134ebff9c Mon Sep 17 00:00:00 2001 From: "Kevin (Kun) \"Kassimo\" Qian" Date: Fri, 28 Sep 2018 22:41:19 -0700 Subject: [PATCH 03/11] Add ts side tests --- tests/015_import_no_ext.out | 13 +++++++++++++ tests/015_import_no_ext.ts | 10 ++++++++++ tests/subdir/mod3.js | 1 + tests/subdir/mod3.ts | 3 +++ tests/subdir/mod4.js | 1 + 5 files changed, 28 insertions(+) create mode 100644 tests/015_import_no_ext.out create mode 100644 tests/015_import_no_ext.ts create mode 100644 tests/subdir/mod3.js create mode 100644 tests/subdir/mod3.ts create mode 100644 tests/subdir/mod4.js diff --git a/tests/015_import_no_ext.out b/tests/015_import_no_ext.out new file mode 100644 index 00000000000000..f4e76f60bfb015 --- /dev/null +++ b/tests/015_import_no_ext.out @@ -0,0 +1,13 @@ +Trying /Users/kevinqian/Desktop/Programming/Deno/deno/tests/015_import_no_ext.ts... +Trying /Users/kevinqian/Desktop/Programming/Deno/deno/tests/subdir/mod3.ts... +Trying /Users/kevinqian/Desktop/Programming/Deno/deno/tests/subdir/mod4.ts... +Trying /Users/kevinqian/Desktop/Programming/Deno/deno/tests/subdir/mod4.js... +Trying http://localhost:4545/tests/subdir/mod2.ts... +Downloading http://localhost:4545/tests/subdir/mod2.ts +Trying /Users/kevinqian/Desktop/Programming/Deno/deno/tests/subdir/print_hello.ts... +Downloading http://localhost:4545/tests/subdir/print_hello.ts +true +[Function: printHello] +[Function: printHello] +true +[Function: printHello] diff --git a/tests/015_import_no_ext.ts b/tests/015_import_no_ext.ts new file mode 100644 index 00000000000000..9bd2acd7e60c05 --- /dev/null +++ b/tests/015_import_no_ext.ts @@ -0,0 +1,10 @@ +import { isTSFile, printHello, phNoExt } from "./subdir/mod3"; +console.log(isTSFile); +console.log(printHello); +console.log(phNoExt); + +import { isMod4 } from "./subdir/mod4"; +console.log(isMod4); + +import { printHello as ph } from "http://localhost:4545/tests/subdir/mod2"; +console.log(ph); diff --git a/tests/subdir/mod3.js b/tests/subdir/mod3.js new file mode 100644 index 00000000000000..ce534f57002711 --- /dev/null +++ b/tests/subdir/mod3.js @@ -0,0 +1 @@ +export const isTSFile = false; diff --git a/tests/subdir/mod3.ts b/tests/subdir/mod3.ts new file mode 100644 index 00000000000000..4f10578bf8d53c --- /dev/null +++ b/tests/subdir/mod3.ts @@ -0,0 +1,3 @@ +export const isTSFile = true; +export { printHello } from "./print_hello.ts"; +export { printHello as phNoExt } from "./print_hello"; diff --git a/tests/subdir/mod4.js b/tests/subdir/mod4.js new file mode 100644 index 00000000000000..71332dbc423043 --- /dev/null +++ b/tests/subdir/mod4.js @@ -0,0 +1 @@ +export const isMod4 = true; From 09739fba74454629cc3a2d9b8867b7b5cda76474 Mon Sep 17 00:00:00 2001 From: "Kevin (Kun) \"Kassimo\" Qian" Date: Fri, 28 Sep 2018 22:55:02 -0700 Subject: [PATCH 04/11] Ooops --- src/deno_dir.rs | 18 ++++++++++-------- tests/015_import_no_ext.out | 6 ------ 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/deno_dir.rs b/src/deno_dir.rs index 5d96b2910a7b3b..a255638ece5348 100644 --- a/src/deno_dir.rs +++ b/src/deno_dir.rs @@ -149,11 +149,12 @@ impl DenoDir { if module_name.ends_with(".ts") || module_name.ends_with(".js") { return try_extension(""); } - println!("Trying {}.ts...", module_name); - if let Ok(v) = try_extension(".ts") { - return Ok(v); + debug!("Trying {}.ts...", module_name); + let ts_attempt = try_extension(".ts"); + if let Ok(_) = ts_attempt { + return ts_attempt; } - println!("Trying {}.js...", module_name); + debug!("Trying {}.js...", module_name); try_extension(".js") } else if module_name.starts_with(ASSET_PREFIX) { panic!("Asset resolution should be done in JS, not Rust."); @@ -176,11 +177,12 @@ impl DenoDir { if module_name.ends_with(".ts") || module_name.ends_with(".js") { return try_extension(""); } - println!("Trying {}.ts...", module_name); - if let Ok(v) = try_extension(".ts") { - return Ok(v); + debug!("Trying {}.ts...", module_name); + let ts_attempt = try_extension(".ts"); + if let Ok(_) = ts_attempt { + return ts_attempt; } - println!("Trying {}.js...", module_name); + debug!("Trying {}.js...", module_name); try_extension(".js") } } diff --git a/tests/015_import_no_ext.out b/tests/015_import_no_ext.out index f4e76f60bfb015..018eb4338a09e2 100644 --- a/tests/015_import_no_ext.out +++ b/tests/015_import_no_ext.out @@ -1,10 +1,4 @@ -Trying /Users/kevinqian/Desktop/Programming/Deno/deno/tests/015_import_no_ext.ts... -Trying /Users/kevinqian/Desktop/Programming/Deno/deno/tests/subdir/mod3.ts... -Trying /Users/kevinqian/Desktop/Programming/Deno/deno/tests/subdir/mod4.ts... -Trying /Users/kevinqian/Desktop/Programming/Deno/deno/tests/subdir/mod4.js... -Trying http://localhost:4545/tests/subdir/mod2.ts... Downloading http://localhost:4545/tests/subdir/mod2.ts -Trying /Users/kevinqian/Desktop/Programming/Deno/deno/tests/subdir/print_hello.ts... Downloading http://localhost:4545/tests/subdir/print_hello.ts true [Function: printHello] From 469771e4e06364172e74041a99f2eca59e4b4d82 Mon Sep 17 00:00:00 2001 From: "Kevin (Kun) \"Kassimo\" Qian" Date: Sat, 29 Sep 2018 18:57:53 -0700 Subject: [PATCH 05/11] Remove duplicate code and simplify --- src/deno_dir.rs | 79 +++++++++++++++++++------------------------------ 1 file changed, 31 insertions(+), 48 deletions(-) diff --git a/src/deno_dir.rs b/src/deno_dir.rs index a255638ece5348..bcb8e0a1eaab58 100644 --- a/src/deno_dir.rs +++ b/src/deno_dir.rs @@ -134,57 +134,40 @@ impl DenoDir { module_name: &str, filename: &str, ) -> DenoResult { - if is_remote(module_name) { - let try_extension = |ext| { - let module_name = format!("{}{}", module_name, ext); - let filename = format!("{}{}", filename, ext); - let source_code = self.fetch_remote_source(&module_name, &filename)?; - return Ok(CodeFetchOutput { - module_name: module_name.to_string(), - filename: filename.to_string(), - source_code, - maybe_output_code: None, - }); - }; - if module_name.ends_with(".ts") || module_name.ends_with(".js") { - return try_extension(""); - } - debug!("Trying {}.ts...", module_name); - let ts_attempt = try_extension(".ts"); - if let Ok(_) = ts_attempt { - return ts_attempt; - } - debug!("Trying {}.js...", module_name); - try_extension(".js") - } else if module_name.starts_with(ASSET_PREFIX) { + if module_name.starts_with(ASSET_PREFIX) { panic!("Asset resolution should be done in JS, not Rust."); - } else { - assert_eq!( - module_name, filename, - "if a module isn't remote, it should have the same filename" - ); - let try_extension = |ext| { - let module_name = format!("{}{}", module_name, ext); - let filename = format!("{}{}", filename, ext); - let source_code = fs::read_to_string(Path::new(&filename))?; - return Ok(CodeFetchOutput { - module_name: module_name.to_string(), - filename: filename.to_string(), - source_code, - maybe_output_code: None, - }); + } + let is_module_remote = is_remote(module_name); + let try_extension = |ext| { + let module_name = format!("{}{}", module_name, ext); + let filename = format!("{}{}", filename, ext); + let source_code = if is_module_remote { + self.fetch_remote_source(&module_name, &filename)? + } else { + assert_eq!( + module_name, filename, + "if a module isn't remote, it should have the same filename" + ); + fs::read_to_string(Path::new(&filename))? }; - if module_name.ends_with(".ts") || module_name.ends_with(".js") { - return try_extension(""); - } - debug!("Trying {}.ts...", module_name); - let ts_attempt = try_extension(".ts"); - if let Ok(_) = ts_attempt { - return ts_attempt; - } - debug!("Trying {}.js...", module_name); - try_extension(".js") + return Ok(CodeFetchOutput { + module_name: module_name.to_string(), + filename: filename.to_string(), + source_code, + maybe_output_code: None, + }); + }; + // Has extension, no guessing required + if module_name.ends_with(".ts") || module_name.ends_with(".js") { + return try_extension(""); + } + debug!("Trying {}.ts...", module_name); + let ts_attempt = try_extension(".ts"); + if let Ok(_) = ts_attempt { + return ts_attempt; } + debug!("Trying {}.js...", module_name); + try_extension(".js") } pub fn code_fetch( From c4f5adb84b84cbf44e5d03ad51829631c103153b Mon Sep 17 00:00:00 2001 From: "Kevin (Kun) \"Kassimo\" Qian" Date: Sun, 30 Sep 2018 12:30:09 -0700 Subject: [PATCH 06/11] Add extension test on fetched code length --- src/deno_dir.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/deno_dir.rs b/src/deno_dir.rs index bcb8e0a1eaab58..c75a7685f0b896 100644 --- a/src/deno_dir.rs +++ b/src/deno_dir.rs @@ -446,11 +446,31 @@ fn test_code_fetch_no_ext() { let r = deno_dir.code_fetch(module_specifier, containing_file); assert!(r.is_ok()); + // Test .ts extension + // Assuming cwd is the deno repo root. + let module_specifier = "./js/main"; + let containing_file = cwd_string.as_str(); + let r = deno_dir.code_fetch(module_specifier, containing_file); + assert!(r.is_ok()); + let code_fetch_output = r.unwrap(); + // could only test .ends_with to avoid include local abs path + assert!(code_fetch_output.module_name.ends_with("/js/main.ts")); + assert!(code_fetch_output.source_code.len() > 10); + + // Test .js extension // Assuming cwd is the deno repo root. let module_specifier = "./js/mock_builtin"; let containing_file = cwd_string.as_str(); let r = deno_dir.code_fetch(module_specifier, containing_file); assert!(r.is_ok()); + let code_fetch_output = r.unwrap(); + // could only test .ends_with to avoid include local abs path + assert!( + code_fetch_output + .module_name + .ends_with("/js/mock_builtin.js") + ); + assert!(code_fetch_output.source_code.len() > 10); } #[test] From 02fbf2394b7fbe4388694c3c8f5f317f10254d61 Mon Sep 17 00:00:00 2001 From: "Kevin (Kun) \"Kassimo\" Qian" Date: Sun, 30 Sep 2018 15:57:33 -0700 Subject: [PATCH 07/11] Use single block_on and add import failure test --- src/http.rs | 49 +++++++++++++++---- ...rt_no_ext.out => 015_import_no_ext.ts.out} | 0 tests/error_006_import_ext_failure.ts | 1 + tests/error_006_import_ext_failure.ts.out | 11 +++++ 4 files changed, 52 insertions(+), 9 deletions(-) rename tests/{015_import_no_ext.out => 015_import_no_ext.ts.out} (100%) create mode 100644 tests/error_006_import_ext_failure.ts create mode 100644 tests/error_006_import_ext_failure.ts.out diff --git a/src/http.rs b/src/http.rs index cbb1ba9b114d59..182a9ff11c4bf9 100644 --- a/src/http.rs +++ b/src/http.rs @@ -1,9 +1,10 @@ // Copyright 2018 the Deno authors. All rights reserved. MIT license. -use errors::DenoResult; +use errors::{DenoError, DenoResult}; +use futures::stream::Concat2; +use futures::{Async, Future}; use tokio_util; -// use futures::Future; use futures::Stream; use hyper; use hyper::client::Client; @@ -27,21 +28,51 @@ pub fn get_client() -> Client { Client::builder().build(c) } +struct FetchedBodyFuture { + body: Concat2, + status: hyper::StatusCode, +} + +struct FetchedBody { + body: hyper::Chunk, + status: hyper::StatusCode, +} + +impl Future for FetchedBodyFuture { + type Item = FetchedBody; + type Error = hyper::Error; + fn poll(&mut self) -> Result, hyper::Error> { + match self.body.poll()? { + Async::Ready(body) => Ok(Async::Ready(FetchedBody { + body, + status: self.status.clone(), + })), + Async::NotReady => Ok(Async::NotReady), + } + } +} + // The CodeFetch message is used to load HTTP javascript resources and expects a // synchronous response, this utility method supports that. pub fn fetch_sync_string(module_name: &str) -> DenoResult { let url = module_name.parse::().unwrap(); let client = get_client(); - let get_future = client.get(url); - let response = tokio_util::block_on(get_future)?; - if !response.status().is_success() { - Err(io::Error::new( + let fetch_future = client.get(url).and_then(|response| { + let status = response.status(); + FetchedBodyFuture { + body: response.into_body().concat2(), + status, + } + }); + + let fetch_result = tokio_util::block_on(fetch_future)?; + if !fetch_result.status.is_success() { + return Err(DenoError::from(io::Error::new( io::ErrorKind::NotFound, format!("cannot load from '{}'", module_name), - ))?; + ))); } - let body = tokio_util::block_on(response.into_body().concat2())?; - Ok(String::from_utf8(body.to_vec()).unwrap()) + Ok(String::from_utf8(fetch_result.body.to_vec()).unwrap()) } /* TODO(ry) Re-enabled this test. Disabling to work around bug in #782. diff --git a/tests/015_import_no_ext.out b/tests/015_import_no_ext.ts.out similarity index 100% rename from tests/015_import_no_ext.out rename to tests/015_import_no_ext.ts.out diff --git a/tests/error_006_import_ext_failure.ts b/tests/error_006_import_ext_failure.ts new file mode 100644 index 00000000000000..3c32303a3456cf --- /dev/null +++ b/tests/error_006_import_ext_failure.ts @@ -0,0 +1 @@ +import "./non-existent"; diff --git a/tests/error_006_import_ext_failure.ts.out b/tests/error_006_import_ext_failure.ts.out new file mode 100644 index 00000000000000..30c9965da7d664 --- /dev/null +++ b/tests/error_006_import_ext_failure.ts.out @@ -0,0 +1,11 @@ +NotFound: Cannot resolve module "./non-existent" from "[WILDCARD]deno/tests/error_006_import_ext_failure.ts" + at maybeError (deno/js/errors.ts:[WILDCARD]) + at maybeThrowError (deno/js/errors.ts:[WILDCARD]) + at sendSync (deno/js/dispatch.ts:[WILDCARD]) + at Object.codeFetch (deno/js/os.ts:[WILDCARD]) + at DenoCompiler.resolveModule (deno/js/compiler.ts:[WILDCARD]) + at DenoCompiler._resolveModuleName (deno/js/compiler.ts:[WILDCARD]) + at moduleNames.map.name (deno/js/compiler.ts:[WILDCARD]) + at Array.map () + at DenoCompiler.resolveModuleNames (deno/js/compiler.ts:[WILDCARD]) + at Object.compilerHost.resolveModuleNames (deno/third_party/node_modules/typescript/lib/typescript.js:[WILDCARD]) From 9da5589ae353e6b0560783fcddd4cc9c62f8067c Mon Sep 17 00:00:00 2001 From: "Kevin (Kun) \"Kassimo\" Qian" Date: Mon, 1 Oct 2018 12:06:09 -0700 Subject: [PATCH 08/11] Forcefully use map to address future problems --- src/deno_dir.rs | 12 +++---- src/http.rs | 65 +++++++++++++++------------------- tests/015_import_no_ext.ts.out | 1 + 3 files changed, 36 insertions(+), 42 deletions(-) diff --git a/src/deno_dir.rs b/src/deno_dir.rs index c75a7685f0b896..fea4bea38f19f7 100644 --- a/src/deno_dir.rs +++ b/src/deno_dir.rs @@ -138,7 +138,7 @@ impl DenoDir { panic!("Asset resolution should be done in JS, not Rust."); } let is_module_remote = is_remote(module_name); - let try_extension = |ext| { + let use_extension = |ext| { let module_name = format!("{}{}", module_name, ext); let filename = format!("{}{}", filename, ext); let source_code = if is_module_remote { @@ -157,17 +157,17 @@ impl DenoDir { maybe_output_code: None, }); }; - // Has extension, no guessing required - if module_name.ends_with(".ts") || module_name.ends_with(".js") { - return try_extension(""); + let default_attempt = use_extension(""); + if let Ok(_) = default_attempt { + return default_attempt; } debug!("Trying {}.ts...", module_name); - let ts_attempt = try_extension(".ts"); + let ts_attempt = use_extension(".ts"); if let Ok(_) = ts_attempt { return ts_attempt; } debug!("Trying {}.js...", module_name); - try_extension(".js") + use_extension(".js") } pub fn code_fetch( diff --git a/src/http.rs b/src/http.rs index 182a9ff11c4bf9..985e6dbda7143e 100644 --- a/src/http.rs +++ b/src/http.rs @@ -1,8 +1,9 @@ // Copyright 2018 the Deno authors. All rights reserved. MIT license. use errors::{DenoError, DenoResult}; -use futures::stream::Concat2; -use futures::{Async, Future}; +use futures; +use futures::future::Either; +use futures::Future; use tokio_util; use futures::Stream; @@ -28,28 +29,27 @@ pub fn get_client() -> Client { Client::builder().build(c) } -struct FetchedBodyFuture { - body: Concat2, - status: hyper::StatusCode, +enum HyperOrIOError { + IO(io::Error), + Hyper(hyper::Error), } -struct FetchedBody { - body: hyper::Chunk, - status: hyper::StatusCode, -} - -impl Future for FetchedBodyFuture { - type Item = FetchedBody; - type Error = hyper::Error; - fn poll(&mut self) -> Result, hyper::Error> { - match self.body.poll()? { - Async::Ready(body) => Ok(Async::Ready(FetchedBody { - body, - status: self.status.clone(), - })), - Async::NotReady => Ok(Async::NotReady), - } +fn response_future( + response: hyper::Response, +) -> impl Future { + if !response.status().is_success() { + return Either::A(futures::future::err(HyperOrIOError::IO(io::Error::new( + io::ErrorKind::NotFound, + format!("module not found"), + )))); } + Either::B( + response + .into_body() + .concat2() + .map(|body| String::from_utf8(body.to_vec()).unwrap()) + .map_err(|err| HyperOrIOError::Hyper(err)), + ) } // The CodeFetch message is used to load HTTP javascript resources and expects a @@ -57,22 +57,15 @@ impl Future for FetchedBodyFuture { pub fn fetch_sync_string(module_name: &str) -> DenoResult { let url = module_name.parse::().unwrap(); let client = get_client(); - let fetch_future = client.get(url).and_then(|response| { - let status = response.status(); - FetchedBodyFuture { - body: response.into_body().concat2(), - status, - } - }); - - let fetch_result = tokio_util::block_on(fetch_future)?; - if !fetch_result.status.is_success() { - return Err(DenoError::from(io::Error::new( - io::ErrorKind::NotFound, - format!("cannot load from '{}'", module_name), - ))); + let fetch_future = client + .get(url) + .map_err(|err| HyperOrIOError::Hyper(err)) + .and_then(response_future); + match tokio_util::block_on(fetch_future) { + Ok(s) => Ok(s), + Err(HyperOrIOError::Hyper(err)) => Err(DenoError::from(err)), + Err(HyperOrIOError::IO(err)) => Err(DenoError::from(err)), } - Ok(String::from_utf8(fetch_result.body.to_vec()).unwrap()) } /* TODO(ry) Re-enabled this test. Disabling to work around bug in #782. diff --git a/tests/015_import_no_ext.ts.out b/tests/015_import_no_ext.ts.out index 018eb4338a09e2..92f21e2a7a881d 100644 --- a/tests/015_import_no_ext.ts.out +++ b/tests/015_import_no_ext.ts.out @@ -1,3 +1,4 @@ +Downloading http://localhost:4545/tests/subdir/mod2 Downloading http://localhost:4545/tests/subdir/mod2.ts Downloading http://localhost:4545/tests/subdir/print_hello.ts true From b9d1cccf2d6691621c970a1b3363339c0b23a681 Mon Sep 17 00:00:00 2001 From: "Kevin (Kun) \"Kassimo\" Qian" Date: Mon, 1 Oct 2018 12:23:12 -0700 Subject: [PATCH 09/11] Drop enum, use DenoError --- src/http.rs | 49 ++++++++++++++++++------------------------------- 1 file changed, 18 insertions(+), 31 deletions(-) diff --git a/src/http.rs b/src/http.rs index 985e6dbda7143e..798b9ad25089e3 100644 --- a/src/http.rs +++ b/src/http.rs @@ -1,5 +1,6 @@ // Copyright 2018 the Deno authors. All rights reserved. MIT license. +use errors; use errors::{DenoError, DenoResult}; use futures; use futures::future::Either; @@ -12,7 +13,6 @@ use hyper::client::Client; use hyper::client::HttpConnector; use hyper::Uri; use hyper_rustls; -use std::io; type Connector = hyper_rustls::HttpsConnector; @@ -29,29 +29,6 @@ pub fn get_client() -> Client { Client::builder().build(c) } -enum HyperOrIOError { - IO(io::Error), - Hyper(hyper::Error), -} - -fn response_future( - response: hyper::Response, -) -> impl Future { - if !response.status().is_success() { - return Either::A(futures::future::err(HyperOrIOError::IO(io::Error::new( - io::ErrorKind::NotFound, - format!("module not found"), - )))); - } - Either::B( - response - .into_body() - .concat2() - .map(|body| String::from_utf8(body.to_vec()).unwrap()) - .map_err(|err| HyperOrIOError::Hyper(err)), - ) -} - // The CodeFetch message is used to load HTTP javascript resources and expects a // synchronous response, this utility method supports that. pub fn fetch_sync_string(module_name: &str) -> DenoResult { @@ -59,13 +36,23 @@ pub fn fetch_sync_string(module_name: &str) -> DenoResult { let client = get_client(); let fetch_future = client .get(url) - .map_err(|err| HyperOrIOError::Hyper(err)) - .and_then(response_future); - match tokio_util::block_on(fetch_future) { - Ok(s) => Ok(s), - Err(HyperOrIOError::Hyper(err)) => Err(DenoError::from(err)), - Err(HyperOrIOError::IO(err)) => Err(DenoError::from(err)), - } + .map_err(|err| DenoError::from(err)) + .and_then(|response| { + if !response.status().is_success() { + return Either::A(futures::future::err(errors::new( + errors::ErrorKind::NotFound, + "module not found".to_string(), + ))); + } + Either::B( + response + .into_body() + .concat2() + .map(|body| String::from_utf8(body.to_vec()).unwrap()) + .map_err(|err| DenoError::from(err)), + ) + }); + tokio_util::block_on(fetch_future) } /* TODO(ry) Re-enabled this test. Disabling to work around bug in #782. From eb95e04a13f681b005d3362120281275fc3551b1 Mon Sep 17 00:00:00 2001 From: "Kevin (Kun) \"Kassimo\" Qian" Date: Mon, 1 Oct 2018 23:14:42 -0700 Subject: [PATCH 10/11] nit --- src/deno_dir.rs | 2 ++ src/http.rs | 7 +++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/deno_dir.rs b/src/deno_dir.rs index fea4bea38f19f7..2801c1f818341e 100644 --- a/src/deno_dir.rs +++ b/src/deno_dir.rs @@ -455,6 +455,7 @@ fn test_code_fetch_no_ext() { let code_fetch_output = r.unwrap(); // could only test .ends_with to avoid include local abs path assert!(code_fetch_output.module_name.ends_with("/js/main.ts")); + assert!(code_fetch_output.filename.ends_with("/js/main.ts")); assert!(code_fetch_output.source_code.len() > 10); // Test .js extension @@ -470,6 +471,7 @@ fn test_code_fetch_no_ext() { .module_name .ends_with("/js/mock_builtin.js") ); + assert!(code_fetch_output.filename.ends_with("/js/mock_builtin.js")); assert!(code_fetch_output.source_code.len() > 10); } diff --git a/src/http.rs b/src/http.rs index 798b9ad25089e3..9ef9adb8f50545 100644 --- a/src/http.rs +++ b/src/http.rs @@ -2,12 +2,11 @@ use errors; use errors::{DenoError, DenoResult}; -use futures; -use futures::future::Either; -use futures::Future; use tokio_util; -use futures::Stream; +use futures; +use futures::future::Either; +use futures::{Future, Stream}; use hyper; use hyper::client::Client; use hyper::client::HttpConnector; From c211eb7b2e880ad89a352618b59d129e4045c877 Mon Sep 17 00:00:00 2001 From: "Kevin (Kun) \"Kassimo\" Qian" Date: Mon, 1 Oct 2018 23:28:39 -0700 Subject: [PATCH 11/11] nit 2 --- src/deno_dir.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/deno_dir.rs b/src/deno_dir.rs index 2801c1f818341e..1e369b40f8fe2c 100644 --- a/src/deno_dir.rs +++ b/src/deno_dir.rs @@ -158,12 +158,12 @@ impl DenoDir { }); }; let default_attempt = use_extension(""); - if let Ok(_) = default_attempt { + if default_attempt.is_ok() { return default_attempt; } debug!("Trying {}.ts...", module_name); let ts_attempt = use_extension(".ts"); - if let Ok(_) = ts_attempt { + if ts_attempt.is_ok() { return ts_attempt; } debug!("Trying {}.js...", module_name);