[api] Make running scripts in AddMessageListener callback work in debug mode

The existance of an `AllowJavascriptExecutionDebugOnly` scope in
`Isolate::ReportPendingMessages()` indicates that the API supports
running arbitrary JS code in a `AddMessageListener` callback.

Currently, this can fail in debug mode: The
`!isolate->external_caught_exception()` condition is checked when
entering API methods inside such a handler. However, if there is
a verbose `TryCatch` active when the exception occurs, this
check fails, and when calling `ToString()` on the exception object
leaves a pending exception itself, the flag is re-set to `true`.

Fix this problem by clearing the flag and the pending exception if
there was one during `ToString()`. This matches the code a few lines
up in `messages.cc`, so the exception state is now consistent
during the callback.

This currently makes a Node.js test fail in debug mode
(`parallel/test-error-reporting`).

Bug: node:7144
Bug: node:17016
Change-Id: I060d00fea3e9a497f4df34c6ff8d6e29ebe96321
Reviewed-on: https://chromium-review.googlesource.com/718096
Commit-Queue: Yang Guo <yangguo@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49466}
This commit is contained in:
Anna Henningsen 2017-11-18 20:15:28 +01:00 committed by Commit Bot
parent 87d7722be4
commit 09b53eef4c
3 changed files with 38 additions and 3 deletions

View File

@ -44,7 +44,7 @@ Alexis Campailla <alexis@janeasystems.com>
Andreas Anyuru <andreas.anyuru@gmail.com>
Andrew Paprocki <andrew@ishiboo.com>
Andrei Kashcha <anvaka@gmail.com>
Anna Henningsen <addaleax@gmail.com>
Anna Henningsen <anna@addaleax.net>
Bangfu Tao <bangfu.tao@samsung.com>
Ben Noordhuis <info@bnoordhuis.nl>
Benjamin Tan <demoneaux@gmail.com>

View File

@ -113,6 +113,9 @@ void MessageHandler::ReportMessage(Isolate* isolate, const MessageLocation* loc,
}
if (!maybe_stringified.ToHandle(&stringified)) {
DCHECK(isolate->has_pending_exception());
isolate->clear_pending_exception();
isolate->set_external_caught_exception(false);
stringified =
isolate->factory()->NewStringFromAsciiChecked("exception");
}

View File

@ -5696,23 +5696,55 @@ TEST(CustomErrorMessage) {
static void check_custom_rethrowing_message(v8::Local<v8::Message> message,
v8::Local<v8::Value> data) {
CHECK(data->IsExternal());
int* callcount = static_cast<int*>(data.As<v8::External>()->Value());
++*callcount;
const char* uncaught_error = "Uncaught exception";
CHECK(message->Get()
->Equals(CcTest::isolate()->GetCurrentContext(),
v8_str(uncaught_error))
.FromJust());
// Test that compiling code inside a message handler works.
CHECK(CompileRunChecked(CcTest::isolate(), "(function(a) { return a; })(42)")
->Equals(CcTest::isolate()->GetCurrentContext(),
v8::Integer::NewFromUnsigned(CcTest::isolate(), 42))
.FromJust());
}
TEST(CustomErrorRethrowsOnToString) {
int callcount = 0;
LocalContext context;
v8::HandleScope scope(context->GetIsolate());
context->GetIsolate()->AddMessageListener(check_custom_rethrowing_message);
v8::Isolate* isolate = context->GetIsolate();
v8::HandleScope scope(isolate);
context->GetIsolate()->AddMessageListener(
check_custom_rethrowing_message, v8::External::New(isolate, &callcount));
CompileRun(
"var e = { toString: function() { throw e; } };"
"try { throw e; } finally {}");
CHECK_EQ(callcount, 1);
context->GetIsolate()->RemoveMessageListeners(
check_custom_rethrowing_message);
}
TEST(CustomErrorRethrowsOnToStringInsideVerboseTryCatch) {
int callcount = 0;
LocalContext context;
v8::Isolate* isolate = context->GetIsolate();
v8::HandleScope scope(isolate);
v8::TryCatch try_catch(isolate);
try_catch.SetVerbose(true);
context->GetIsolate()->AddMessageListener(
check_custom_rethrowing_message, v8::External::New(isolate, &callcount));
CompileRun(
"var e = { toString: function() { throw e; } };"
"try { throw e; } finally {}");
CHECK_EQ(callcount, 1);
context->GetIsolate()->RemoveMessageListeners(
check_custom_rethrowing_message);
}