diff options
-rw-r--r-- | core/isolate.rs | 48 | ||||
-rw-r--r-- | core/libdeno.rs | 5 | ||||
-rw-r--r-- | core/libdeno/deno.h | 10 | ||||
-rw-r--r-- | core/libdeno/exceptions.cc | 10 | ||||
-rw-r--r-- | core/libdeno/exceptions.h | 2 | ||||
-rw-r--r-- | core/libdeno/internal.h | 2 | ||||
-rw-r--r-- | core/libdeno/modules.cc | 52 | ||||
-rw-r--r-- | core/libdeno/modules_test.cc | 132 |
8 files changed, 218 insertions, 43 deletions
diff --git a/core/isolate.rs b/core/isolate.rs index af9d07f75..8ad8ef8b2 100644 --- a/core/isolate.rs +++ b/core/isolate.rs @@ -80,7 +80,8 @@ pub type OpResult<E> = Result<Op<E>, E>; type CoreDispatchFn = dyn Fn(&[u8], Option<PinnedBuf>) -> CoreOp; -pub type DynImportFuture = Box<dyn Future<Item = deno_mod, Error = ()> + Send>; +pub type DynImportFuture = + Box<dyn Future<Item = deno_mod, Error = ErrBox> + Send>; type DynImportFn = dyn Fn(&str, &str) -> DynImportFuture; type JSErrorCreateFn = dyn Fn(V8Exception) -> ErrBox; @@ -94,13 +95,12 @@ struct DynImport { impl Future for DynImport { type Item = (deno_dyn_import_id, deno_mod); - type Error = (); - fn poll(&mut self) -> Poll<Self::Item, ()> { + type Error = (deno_mod, ErrBox); + fn poll(&mut self) -> Poll<Self::Item, Self::Error> { match self.inner.poll() { Ok(Ready(mod_id)) => Ok(Ready((self.id, mod_id))), Ok(NotReady) => Ok(NotReady), - // Note that mod_id 0 indicates error. - Err(()) => Ok(Ready((self.id, 0))), + Err(e) => Err((self.id, e)), } } } @@ -437,15 +437,24 @@ impl Isolate { fn dyn_import_done( &self, id: libdeno::deno_dyn_import_id, - mod_id: deno_mod, + result: Result<deno_mod, Option<String>>, ) -> Result<(), ErrBox> { - debug!("dyn_import_done {} {}", id, mod_id); + debug!("dyn_import_done {} {:?}", id, result); + let (mod_id, maybe_err_str) = match result { + Ok(mod_id) => (mod_id, None), + Err(None) => (0, None), + Err(Some(err_str)) => (0, Some(err_str)), + }; + let err_ptr = maybe_err_str + .map(|e| e.as_ptr() as *const c_char) + .unwrap_or(std::ptr::null()); unsafe { - libdeno::deno_dyn_import( + libdeno::deno_dyn_import_done( self.libdeno_isolate, self.as_raw_ptr(), id, mod_id, + err_ptr, ) }; self.check_last_exception().map_err(|err| { @@ -556,13 +565,18 @@ impl Future for Isolate { loop { // If there are any pending dyn_import futures, do those first. - match self.pending_dyn_imports.poll() { - Err(()) => unreachable!(), - Ok(NotReady) => unreachable!(), - Ok(Ready(None)) => (), - Ok(Ready(Some((dyn_import_id, mod_id)))) => { - self.dyn_import_done(dyn_import_id, mod_id)?; - continue; + loop { + match self.pending_dyn_imports.poll() { + Ok(NotReady) | Ok(Ready(None)) => break, + Ok(Ready(Some((dyn_import_id, mod_id)))) => { + match self.mod_evaluate(mod_id) { + Ok(()) => self.dyn_import_done(dyn_import_id, Ok(mod_id))?, + Err(..) => self.dyn_import_done(dyn_import_id, Err(None))?, + } + } + Err((dyn_import_id, err)) => { + self.dyn_import_done(dyn_import_id, Err(Some(err.to_string())))? + } } } @@ -904,7 +918,9 @@ pub mod tests { count_.fetch_add(1, Ordering::Relaxed); assert_eq!(specifier, "foo.js"); assert_eq!(referrer, "dyn_import2.js"); - Box::new(futures::future::err(())) + Box::new(futures::future::err( + std::io::Error::new(std::io::ErrorKind::Other, "oh no!").into(), + )) }); js_check(isolate.execute( "dyn_import2.js", diff --git a/core/libdeno.rs b/core/libdeno.rs index 84f21e89e..c402d8754 100644 --- a/core/libdeno.rs +++ b/core/libdeno.rs @@ -193,7 +193,7 @@ type deno_recv_cb = unsafe extern "C" fn( ); /// Called when dynamic import is called in JS: import('foo') -/// Embedder must call deno_dyn_import() with the specified id and +/// Embedder must call deno_dyn_import_done() with the specified id and /// the module. #[allow(non_camel_case_types)] type deno_dyn_import_cb = unsafe extern "C" fn( @@ -308,11 +308,12 @@ extern "C" { ); /// Call exactly once for every deno_dyn_import_cb. - pub fn deno_dyn_import( + pub fn deno_dyn_import_done( i: *const isolate, user_data: *const c_void, id: deno_dyn_import_id, mod_id: deno_mod, + error_str: *const c_char, ); pub fn deno_snapshot_new(i: *const isolate) -> Snapshot1<'static>; diff --git a/core/libdeno/deno.h b/core/libdeno/deno.h index 745285554..fe5214848 100644 --- a/core/libdeno/deno.h +++ b/core/libdeno/deno.h @@ -36,7 +36,7 @@ typedef void (*deno_recv_cb)(void* user_data, deno_buf control_buf, typedef int deno_dyn_import_id; // Called when dynamic import is called in JS: import('foo') -// Embedder must call deno_dyn_import() with the specified id and +// Embedder must call deno_dyn_import_done() with the specified id and // the module. typedef void (*deno_dyn_import_cb)(void* user_data, const char* specifier, const char* referrer, deno_dyn_import_id id); @@ -127,8 +127,12 @@ void deno_mod_evaluate(Deno* d, void* user_data, deno_mod id); // Call exactly once for every deno_dyn_import_cb. // Note this call will execute JS. -void deno_dyn_import(Deno* d, void* user_data, deno_dyn_import_id id, - deno_mod mod_id); +// Either mod_id is zero and error_str is not null OR mod_id is valid and +// error_str is null. +// TODO(ry) The errors arising from dynamic import are not exactly the same as +// those arising from ops in Deno. +void deno_dyn_import_done(Deno* d, void* user_data, deno_dyn_import_id id, + deno_mod mod_id, const char* error_str); #ifdef __cplusplus } // extern "C" diff --git a/core/libdeno/exceptions.cc b/core/libdeno/exceptions.cc index 7484e4eb4..5f4d578b6 100644 --- a/core/libdeno/exceptions.cc +++ b/core/libdeno/exceptions.cc @@ -200,6 +200,7 @@ void HandleException(v8::Local<v8::Context> context, std::string json_str = EncodeExceptionAsJSON(context, exception); CHECK_NOT_NULL(d); d->last_exception_ = json_str; + d->last_exception_handle_.Reset(isolate, exception); } void HandleExceptionMessage(v8::Local<v8::Context> context, @@ -218,6 +219,15 @@ void HandleExceptionMessage(v8::Local<v8::Context> context, d->last_exception_ = json_str; } +void ClearException(v8::Local<v8::Context> context) { + v8::Isolate* isolate = context->GetIsolate(); + DenoIsolate* d = DenoIsolate::FromIsolate(isolate); + CHECK_NOT_NULL(d); + + d->last_exception_.clear(); + d->last_exception_handle_.Reset(); +} + void ThrowInvalidArgument(v8::Isolate* isolate) { isolate->ThrowException(v8::Exception::TypeError(v8_str("Invalid Argument"))); } diff --git a/core/libdeno/exceptions.h b/core/libdeno/exceptions.h index 413bcb7ef..2c0947be2 100644 --- a/core/libdeno/exceptions.h +++ b/core/libdeno/exceptions.h @@ -19,6 +19,8 @@ void HandleException(v8::Local<v8::Context> context, void HandleExceptionMessage(v8::Local<v8::Context> context, v8::Local<v8::Message> message); +void ClearException(v8::Local<v8::Context> context); + void ThrowInvalidArgument(v8::Isolate* isolate); } // namespace deno diff --git a/core/libdeno/internal.h b/core/libdeno/internal.h index 7702c3a16..50e85017e 100644 --- a/core/libdeno/internal.h +++ b/core/libdeno/internal.h @@ -51,6 +51,7 @@ class DenoIsolate { } ~DenoIsolate() { + last_exception_handle_.Reset(); shared_ab_.Reset(); if (locker_) { delete locker_; @@ -111,6 +112,7 @@ class DenoIsolate { v8::Persistent<v8::Context> context_; std::map<int, v8::Persistent<v8::Value>> pending_promise_map_; std::string last_exception_; + v8::Persistent<v8::Value> last_exception_handle_; v8::Persistent<v8::Function> recv_; v8::StartupData snapshot_; v8::Persistent<v8::ArrayBuffer> global_import_buf_; diff --git a/core/libdeno/modules.cc b/core/libdeno/modules.cc index 77b330c3e..0d3a9c70f 100644 --- a/core/libdeno/modules.cc +++ b/core/libdeno/modules.cc @@ -2,6 +2,7 @@ #include "exceptions.h" #include "internal.h" +using deno::ClearException; using deno::DenoIsolate; using deno::HandleException; using v8::Boolean; @@ -140,20 +141,35 @@ void deno_mod_evaluate(Deno* d_, void* user_data, deno_mod id) { v8::Context::Scope context_scope(context); auto* info = d->GetModuleInfo(id); - Local<Module> module = info->handle.Get(isolate); - - CHECK_EQ(Module::kInstantiated, module->GetStatus()); + auto module = info->handle.Get(isolate); + auto status = module->GetStatus(); + + if (status == Module::kInstantiated) { + bool ok = !module->Evaluate(context).IsEmpty(); + status = module->GetStatus(); // Update status after evaluating. + CHECK_IMPLIES(ok, status == Module::kEvaluated); + CHECK_IMPLIES(!ok, status == Module::kErrored); + } - auto maybe_result = module->Evaluate(context); - if (maybe_result.IsEmpty()) { - CHECK_EQ(Module::kErrored, module->GetStatus()); - HandleException(context, module->GetException()); + switch (status) { + case Module::kEvaluated: + ClearException(context); + break; + case Module::kErrored: + HandleException(context, module->GetException()); + break; + default: + FATAL("Unexpected module status: %d", static_cast<int>(status)); } } -void deno_dyn_import(Deno* d_, void* user_data, deno_dyn_import_id import_id, - deno_mod mod_id) { +void deno_dyn_import_done(Deno* d_, void* user_data, + deno_dyn_import_id import_id, deno_mod mod_id, + const char* error_str) { auto* d = unwrap(d_); + CHECK((mod_id == 0 && error_str != nullptr) || + (mod_id != 0 && error_str == nullptr) || + (mod_id == 0 && !d->last_exception_handle_.IsEmpty())); deno::UserDataScope user_data_scope(d, user_data); auto* isolate = d->isolate_; @@ -163,8 +179,6 @@ void deno_dyn_import(Deno* d_, void* user_data, deno_dyn_import_id import_id, auto context = d->context_.Get(d->isolate_); v8::Context::Scope context_scope(context); - v8::TryCatch try_catch(isolate); - auto it = d->dyn_import_map_.find(import_id); if (it == d->dyn_import_map_.end()) { CHECK(false); // TODO(ry) error on bad import_id. @@ -183,18 +197,22 @@ void deno_dyn_import(Deno* d_, void* user_data, deno_dyn_import_id import_id, if (info == nullptr) { // Resolution error. - promise->Reject(context, v8::Null(isolate)).ToChecked(); + if (error_str != nullptr) { + auto msg = deno::v8_str(error_str); + auto exception = v8::Exception::TypeError(msg); + promise->Reject(context, exception).ToChecked(); + } else { + auto e = d->last_exception_handle_.Get(isolate); + ClearException(context); + promise->Reject(context, e).ToChecked(); + } } else { // Resolution success Local<Module> module = info->handle.Get(isolate); - CHECK_GE(module->GetStatus(), v8::Module::kInstantiated); + CHECK_EQ(module->GetStatus(), v8::Module::kEvaluated); Local<Value> module_namespace = module->GetModuleNamespace(); promise->Resolve(context, module_namespace).ToChecked(); } - - if (try_catch.HasCaught()) { - HandleException(context, try_catch.Exception()); - } } } // extern "C" diff --git a/core/libdeno/modules_test.cc b/core/libdeno/modules_test.cc index 490dccdbe..987e88791 100644 --- a/core/libdeno/modules_test.cc +++ b/core/libdeno/modules_test.cc @@ -158,7 +158,7 @@ TEST(ModulesTest, DynamicImportSuccess) { dyn_import_count++; EXPECT_STREQ(specifier, "foo"); EXPECT_STREQ(referrer, "a.js"); - deno_dyn_import(d, d, import_id, b); + deno_dyn_import_done(d, d, import_id, b, nullptr); }; const char* src = "(async () => { \n" @@ -179,6 +179,8 @@ TEST(ModulesTest, DynamicImportSuccess) { EXPECT_EQ(nullptr, deno_last_exception(d)); deno_mod_instantiate(d, d, b, nullptr); EXPECT_EQ(nullptr, deno_last_exception(d)); + deno_mod_evaluate(d, d, b); + EXPECT_EQ(nullptr, deno_last_exception(d)); deno_mod_evaluate(d, d, a); EXPECT_EQ(nullptr, deno_last_exception(d)); deno_check_promise_errors(d); @@ -198,7 +200,7 @@ TEST(ModulesTest, DynamicImportError) { EXPECT_STREQ(specifier, "foo"); EXPECT_STREQ(referrer, "a.js"); // We indicate there was an error resolving by returning mod_id 0. - deno_dyn_import(d, d, import_id, 0); + deno_dyn_import_done(d, d, import_id, 0, "foo not found"); }; const char* src = "(async () => { \n" @@ -218,6 +220,8 @@ TEST(ModulesTest, DynamicImportError) { // Now we should get an error. deno_check_promise_errors(d); EXPECT_NE(deno_last_exception(d), nullptr); + std::string e(deno_last_exception(d)); + EXPECT_NE(e.find("Uncaught TypeError: foo not found"), std::string::npos); deno_delete(d); EXPECT_EQ(0, exec_count); EXPECT_EQ(1, dyn_import_count); @@ -234,7 +238,7 @@ TEST(ModulesTest, DynamicImportAsync) { dyn_import_count++; EXPECT_STREQ(specifier, "foo"); EXPECT_STREQ(referrer, "a.js"); - // We don't call deno_dyn_import until later. + // We don't call deno_dyn_import_done until later. import_ids.push_back(import_id); }; const char* src = @@ -270,13 +274,15 @@ TEST(ModulesTest, DynamicImportAsync) { EXPECT_EQ(nullptr, deno_last_exception(d)); deno_mod_instantiate(d, d, b, nullptr); EXPECT_EQ(nullptr, deno_last_exception(d)); + deno_mod_evaluate(d, d, b); + EXPECT_EQ(nullptr, deno_last_exception(d)); // Now we resolve the import. EXPECT_EQ(1u, import_ids.size()); auto import_id = import_ids.back(); import_ids.pop_back(); - deno_dyn_import(d, d, import_id, b); + deno_dyn_import_done(d, d, import_id, b, nullptr); EXPECT_EQ(nullptr, deno_last_exception(d)); deno_check_promise_errors(d); @@ -288,7 +294,7 @@ TEST(ModulesTest, DynamicImportAsync) { import_id = import_ids.back(); import_ids.pop_back(); - deno_dyn_import(d, d, import_id, b); + deno_dyn_import_done(d, d, import_id, b, nullptr); EXPECT_EQ(nullptr, deno_last_exception(d)); deno_check_promise_errors(d); @@ -300,3 +306,119 @@ TEST(ModulesTest, DynamicImportAsync) { deno_delete(d); } + +TEST(ModulesTest, DynamicImportThrows) { + exec_count = 0; + static int dyn_import_count = 0; + static deno_mod b = 0; + static std::vector<deno_dyn_import_id> import_ids = {}; + auto dyn_import_cb = [](auto user_data, const char* specifier, + const char* referrer, deno_dyn_import_id import_id) { + dyn_import_count++; + EXPECT_STREQ(specifier, "b.js"); + EXPECT_STREQ(referrer, "a.js"); + // We don't call deno_dyn_import_done until later. + import_ids.push_back(import_id); + }; + Deno* d = deno_new(deno_config{0, snapshot, empty, recv_cb, dyn_import_cb}); + + // Instantiate and evaluate the root module. This should succeed. + const char* a_src = + "(async () => { \n" + " let mod = await import('b.js'); \n" + // unreachable + " Deno.core.send(new Uint8Array([4])); \n" + "})(); \n"; + static deno_mod a = deno_mod_new(d, true, "a.js", a_src); + EXPECT_NE(a, 0); + EXPECT_EQ(nullptr, deno_last_exception(d)); + deno_mod_instantiate(d, d, a, nullptr); + EXPECT_EQ(nullptr, deno_last_exception(d)); + deno_mod_evaluate(d, d, a); + EXPECT_EQ(nullptr, deno_last_exception(d)); + deno_check_promise_errors(d); + EXPECT_EQ(deno_last_exception(d), nullptr); + + // Instantiate b.js, which should succeed. + const char* b_src = "throw new Error('foo')"; + b = deno_mod_new(d, false, "b.js", b_src); + EXPECT_NE(b, 0); + EXPECT_EQ(nullptr, deno_last_exception(d)); + deno_mod_instantiate(d, d, b, nullptr); + EXPECT_EQ(nullptr, deno_last_exception(d)); + + // Evaluate b.js. It throws in the global scope, so deno_last_exception() + // should be non-null afterwards. + deno_mod_evaluate(d, d, b); + EXPECT_NE(deno_last_exception(d), nullptr); + + // Resolve the dynamic import of b.js. Since deno_mod_evaluate() failed, + // we indicate failure to deno_dyn_import_done() by setting mod_id to 0. + // The last error should be picked up and cleared by deno_dyn_import_done(). + EXPECT_EQ(1u, import_ids.size()); + auto import_id = import_ids.back(); + import_ids.pop_back(); + deno_dyn_import_done(d, d, import_id, 0, nullptr); + EXPECT_EQ(deno_last_exception(d), nullptr); + + // Since the dynamically imported module threw an error, + // it should show up as an unhandled promise rejection. + deno_check_promise_errors(d); + EXPECT_NE(deno_last_exception(d), nullptr); + std::string e(deno_last_exception(d)); + EXPECT_NE(e.find("Uncaught Error: foo"), std::string::npos); + + EXPECT_EQ(1, dyn_import_count); + EXPECT_EQ(0, exec_count); + + deno_delete(d); +} + +TEST(ModulesTest, DynamicImportSyntaxError) { + exec_count = 0; + static int dyn_import_count = 0; + auto dyn_import_cb = [](auto user_data, const char* specifier, + const char* referrer, deno_dyn_import_id import_id) { + auto d = reinterpret_cast<Deno*>(user_data); + dyn_import_count++; + EXPECT_STREQ(specifier, "b.js"); + EXPECT_STREQ(referrer, "a.js"); + + // Compile b.js, which should fail because of the syntax error. + deno_mod b = deno_mod_new(d, false, "b.js", "syntax error"); + EXPECT_EQ(b, 0); + EXPECT_NE(nullptr, deno_last_exception(d)); + + // `deno_dyn_import_done` should consume the last exception, and use it + // to reject the dynamic import promise. + deno_dyn_import_done(d, d, import_id, 0, nullptr); + EXPECT_EQ(nullptr, deno_last_exception(d)); + }; + Deno* d = deno_new(deno_config{0, snapshot, empty, recv_cb, dyn_import_cb}); + + // Instantiate and evaluate the root module. This should succeed. + const char* src = + "(async () => { \n" + " let mod = await import('b.js'); \n" + // unreachable + " Deno.core.send(new Uint8Array([4])); \n" + "})(); \n"; + static deno_mod a = deno_mod_new(d, true, "a.js", src); + EXPECT_NE(a, 0); + EXPECT_EQ(nullptr, deno_last_exception(d)); + deno_mod_instantiate(d, d, a, nullptr); + EXPECT_EQ(nullptr, deno_last_exception(d)); + deno_mod_evaluate(d, d, a); + EXPECT_EQ(nullptr, deno_last_exception(d)); + + // The failed dynamic import should cause an unhandled promise rejection. + deno_check_promise_errors(d); + EXPECT_NE(deno_last_exception(d), nullptr); + EXPECT_NE(std::string(deno_last_exception(d)).find("Syntax"), + std::string::npos); + + EXPECT_EQ(1, dyn_import_count); + EXPECT_EQ(0, exec_count); + + deno_delete(d); +} |