Make exception thrown via v8 public API propagate to v8::TryCatch as JS thrown exceptions do.

src/messages.cc

Index: src/messages.cc
diff --git a/src/messages.cc b/src/messages.cc
index 42fc3c9bd66fdd2742983340db3b41b0ad158114..91467ab5b9542655304e12442cdb73be31d6aba0 100644
--- a/src/messages.cc
+++ b/src/messages.cc
@@ -135,6 +135,9 @@ void MessageHandler::ReportMessage(MessageLocation* loc,
   if (global_length == 0) {
     DefaultMessageReport(loc, message);
   } else {
+    // We are calling into embedder's code.  Clean pending exception
+    // and ignore scheduled exceptions the callback can throw.
+    Top::clear_pending_exception();

[Mads Ager (chromium), 2011/01/28 11:39:18]

Should this be moved to the caller? I think we only call this if there is
pending exception. In the caller I think we always set the pending_exception
back at the end. So, in the caller, maybe we should clear the pending exception
before calling this?

If not, I think the comment should be extended to state that this is only called
from a place where the pending exception is restored on return?

Ignoring new exceptions while report the current one seems like the right thing
to do.

[antonm, 2011/01/28 13:37:25]

Let me try modified second approach: I'll move all the logic of thread_local_
save and restore over here, does that look reasonable?

On 2011/01/28 11:39:18, Mads Ager wrote:

     for (int i = 0; i < global_length; i++) {
       HandleScope scope;
       if (global_listeners.get(i)->IsUndefined()) continue;
@@ -144,6 +147,10 @@ void MessageHandler::ReportMessage(MessageLocation* loc,
           FUNCTION_CAST<v8::MessageCallback>(callback_obj->proxy());
       Handle<Object> callback_data(listener.get(1));
       callback(api_message_obj, v8::Utils::ToLocal(callback_data));
+      if (Top::has_scheduled_exception()) {
+        // Consider logging it somehow.

[Vitaly Repeshko, 2011/01/28 14:03:33]

What if we get an OOM or termination exception here? There are probably not safe
to ignore.

[antonm, 2011/01/28 14:43:18]

That sounds like a very good point.  I think we should be more or less safe for
OOM which will mark the context, but termination exception might be clobbered
here.

But let's investigate and address it later, ok?  Before my change throwing
exception from callbacks would just crash/trigger asserts in v8 anyway.

+        Top::clear_scheduled_exception();
+      }
     }
   }
 }