Skip to content

Commit

Permalink
Add error handling for dynamic imports not found
Browse files Browse the repository at this point in the history
  • Loading branch information
@piscisaureus
piscisaureus committed Jul 23, 2019
1 parent e49d1e1 commit 92f5e59
Show file tree
Hide file tree
Showing 5 changed files with 160 additions and 18 deletions.
6 changes: 5 additions & 1 deletion core/libdeno/deno.h
View file
Expand Up @@ -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.
// 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(Deno* d, void* user_data, deno_dyn_import_id id,
deno_mod mod_id);
deno_mod mod_id, const char* error_str);

#ifdef __cplusplus
} // extern "C"
Expand Down
1 change: 1 addition & 0 deletions core/libdeno/exceptions.cc
View file
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions core/libdeno/internal.h
View file
Expand Up @@ -111,6 +111,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_;
Expand Down
38 changes: 25 additions & 13 deletions core/libdeno/modules.cc
View file
Expand Up @@ -152,8 +152,11 @@ void deno_mod_evaluate(Deno* d_, void* user_data, deno_mod id) {
}

void deno_dyn_import(Deno* d_, void* user_data, deno_dyn_import_id import_id,
deno_mod mod_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_;
Expand All @@ -163,8 +166,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.
Expand All @@ -182,19 +183,30 @@ void deno_dyn_import(Deno* d_, void* user_data, deno_dyn_import_id import_id,
d->dyn_import_map_.erase(it);

if (info == nullptr) {
// Resolution error.
promise->Reject(context, v8::Null(isolate)).ToChecked();
} else {
// Resolution success
Local<Module> module = info->handle.Get(isolate);
CHECK_GE(module->GetStatus(), v8::Module::kInstantiated);
Local<Value> module_namespace = module->GetModuleNamespace();
promise->Resolve(context, module_namespace).ToChecked();
if (error_str != nullptr) {
// Resolution error.
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);
promise->Reject(context, e).ToChecked();
}
return;
}

if (try_catch.HasCaught()) {
HandleException(context, try_catch.Exception());
// Resolution success
Local<Module> module = info->handle.Get(isolate);

if (module->GetStatus() == v8::Module::Status::kErrored) {
auto e = module->GetException();
promise->Reject(context, e).ToChecked();
return;
}

CHECK_GE(module->GetStatus(), v8::Module::kEvaluated);
Local<Value> module_namespace = module->GetModuleNamespace();
promise->Resolve(context, module_namespace).ToChecked();
}

} // extern "C"
132 changes: 128 additions & 4 deletions core/libdeno/modules_test.cc
View file
Expand Up @@ -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(d, d, import_id, b, nullptr);
};
const char* src =
"(async () => { \n"
Expand All @@ -179,6 +179,10 @@ 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);
Expand All @@ -198,7 +202,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(d, d, import_id, 0, "foo not found");
};
const char* src =
"(async () => { \n"
Expand All @@ -217,7 +221,11 @@ TEST(ModulesTest, DynamicImportError) {
EXPECT_EQ(nullptr, deno_last_exception(d));
// 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);
Expand Down Expand Up @@ -270,13 +278,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(d, d, import_id, b, nullptr);

EXPECT_EQ(nullptr, deno_last_exception(d));
deno_check_promise_errors(d);
Expand All @@ -288,7 +298,7 @@ TEST(ModulesTest, DynamicImportAsync) {

import_id = import_ids.back();
import_ids.pop_back();
deno_dyn_import(d, d, import_id, b);
deno_dyn_import(d, d, import_id, b, nullptr);

EXPECT_EQ(nullptr, deno_last_exception(d));
deno_check_promise_errors(d);
Expand All @@ -300,3 +310,117 @@ 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) {
// auto d = reinterpret_cast<Deno*>(user_data);
dyn_import_count++;
EXPECT_STREQ(specifier, "b.js");
EXPECT_STREQ(referrer, "a.js");
// We don't call deno_dyn_import until later.
import_ids.push_back(import_id);
};
const char* src =
"(async () => { \n"
" let mod = await import('b.js'); \n"
// unreachable
" Deno.core.send(new Uint8Array([4])); \n"
"})(); \n";
Deno* d = deno_new(deno_config{0, snapshot, empty, recv_cb, dyn_import_cb});
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));

// Evaluate. We check that there are no errors, and Deno.core.send has not
// been called.
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);
EXPECT_EQ(0, exec_count);
EXPECT_EQ(1, dyn_import_count);

// Instantiate b.js
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));

deno_mod_evaluate(d, d, b);
EXPECT_NE(deno_last_exception(d), nullptr);

// Now we resolve the import.
EXPECT_EQ(1u, import_ids.size());
auto import_id = import_ids.back();
import_ids.pop_back();

EXPECT_EQ(0, exec_count);

deno_dyn_import(d, d, import_id, b, nullptr);

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;
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) {
auto d = reinterpret_cast<Deno*>(user_data);
dyn_import_count++;
EXPECT_STREQ(specifier, "b.js");
EXPECT_STREQ(referrer, "a.js");

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(d, d, import_id, 0, nullptr);
};
const char* src =
"(async () => { \n"
" let mod = await import('b.js'); \n"
// unreachable
" Deno.core.send(new Uint8Array([4])); \n"
"})(); \n";
Deno* d = deno_new(deno_config{0, snapshot, empty, recv_cb, dyn_import_cb});
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));

// Evaluate. We check that there are no errors, and Deno.core.send has not
// been called.
deno_mod_evaluate(d, d, a);

// EXPECT_EQ(nullptr, deno_last_exception(d));
// deno_check_promise_errors(d);
EXPECT_NE(deno_last_exception(d), nullptr);
std::string e(deno_last_exception(d));
EXPECT_NE(e.find("Syntax"), std::string::npos);

EXPECT_EQ(1, dyn_import_count);
EXPECT_EQ(0, exec_count);

deno_delete(d);
}

0 comments on commit 92f5e59

Please sign in to comment.