Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(156)

Unified Diff: Source/bindings/core/v8/V8Initializer.cpp

Issue 625943002: Catch uncaught promise rejections from V8 and log to console. (Closed) Base URL: svn://svn.chromium.org/blink/trunk
Patch Set: Created 6 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: Source/bindings/core/v8/V8Initializer.cpp
diff --git a/Source/bindings/core/v8/V8Initializer.cpp b/Source/bindings/core/v8/V8Initializer.cpp
index f73978540a245000a78a8a178ab3b90a6fb93a5a..f78495db3857812b80a4685cef56a38b1cc646fd 100644
--- a/Source/bindings/core/v8/V8Initializer.cpp
+++ b/Source/bindings/core/v8/V8Initializer.cpp
@@ -30,6 +30,8 @@
#include "bindings/core/v8/ScriptCallStackFactory.h"
#include "bindings/core/v8/ScriptController.h"
#include "bindings/core/v8/ScriptProfiler.h"
+#include "bindings/core/v8/ScriptValue.h"
+#include "bindings/core/v8/ScriptValueTraits.h"
#include "bindings/core/v8/V8Binding.h"
#include "bindings/core/v8/V8DOMException.h"
#include "bindings/core/v8/V8ErrorEvent.h"
@@ -41,6 +43,7 @@
#include "bindings/core/v8/V8Window.h"
#include "core/dom/Document.h"
#include "core/dom/ExceptionCode.h"
+#include "core/dom/Microtask.h"
#include "core/frame/ConsoleTypes.h"
#include "core/frame/LocalDOMWindow.h"
#include "core/frame/LocalFrame.h"
@@ -155,6 +158,125 @@ static void messageHandlerInMainThread(v8::Handle<v8::Message> message, v8::Hand
}
}
+namespace {
+
+struct PromiseRejectMessage {
yurys 2014/10/03 15:08:06 class?
aandrey 2014/10/06 10:52:57 Done.
+ PromiseRejectMessage(const ScriptValue& promise, const ScriptValue& rejectValue)
+ : m_promise(promise)
+ , m_rejectValue(rejectValue)
+ {
+ }
+
+ ScriptValue m_promise;
+ ScriptValue m_rejectValue;
+};
+
+} // namespace
+
+typedef WillBeHeapHashMap<ScriptValue, OwnPtr<PromiseRejectMessage>, ScriptValueHash, ScriptValueHashTraits> PromiseRejectMessageMap;
+static bool promiseEndOfTurnTaskScheduled = false;
+
+static PromiseRejectMessageMap& promiseRejectMessageQueue()
+{
+ DEFINE_STATIC_LOCAL(OwnPtrWillBePersistent<PromiseRejectMessageMap>, map, (adoptPtrWillBeNoop(new PromiseRejectMessageMap())));
+ return *map;
+}
+
+static void firePromiseRejectMessages()
yurys 2014/10/03 15:08:06 Please wrap the new code into a class.
aandrey 2014/10/06 10:52:57 I'd rather put this code together with the excepti
+{
+ ASSERT(isMainThread());
+
+ promiseEndOfTurnTaskScheduled = false;
+ v8::Isolate::GetCurrent()->RemoveCallCompletedCallback(&firePromiseRejectMessages);
yurys 2014/10/03 15:08:06 Why not add this callback once on the isolate init
aandrey 2014/10/06 10:52:57 So that we should not affect common use cases: we
+
+ // FIXME: Send more data from V8.
+ int scriptId = 0;
+ AccessControlStatus corsStatus = SharableCrossOrigin;
+ Vector<ScriptCallFrame> callFrames;
+ RefPtrWillBeRawPtr<ScriptCallStack> callStack = ScriptCallStack::create(callFrames);
yurys 2014/10/03 15:08:05 = nullptr?
aandrey 2014/10/06 10:52:57 Done.
+
+ PromiseRejectMessageMap& map = promiseRejectMessageQueue();
+ while (!map.isEmpty()) {
+ PromiseRejectMessageMap::iterator it = map.begin();
+ ScriptValue promise = it->value->m_promise;
+ ScriptValue rejectValue = it->value->m_rejectValue;
+ map.remove(it);
+
+ ScriptState* scriptState = promise.scriptState();
+ if (scriptState->contextIsValid())
+ continue;
+ ScriptState::Scope scope(scriptState);
+
+ RefPtrWillBeRawPtr<ErrorEvent> event = ErrorEvent::createSanitizedError(&scriptState->world());
+ event->setUnsanitizedMessage("Uncaught promise reject");
+
+ v8::Handle<v8::Value> data = rejectValue.v8Value();
yurys 2014/10/03 15:08:06 The whole idea we agreed on was to pass rejected v
aandrey 2014/10/06 10:52:57 Done. Passing ScriptArguments.
+ if (!data.IsEmpty() && V8DOMWrapper::isDOMWrapper(data)) {
+ v8::Handle<v8::Object> obj = v8::Handle<v8::Object>::Cast(data);
+ const WrapperTypeInfo* type = toWrapperTypeInfo(obj);
+ if (V8DOMException::wrapperTypeInfo.isSubclass(type)) {
+ DOMException* exception = V8DOMException::toImpl(obj);
+ if (exception && !exception->messageForConsole().isEmpty())
+ event->setUnsanitizedMessage("Uncaught promise reject " + exception->toStringForConsole());
+ }
+ }
+
+ if (scriptState->world().isPrivateScriptIsolatedWorld()) {
+ // We allow a private script to dispatch error events even in a EventDispatchForbiddenScope scope.
+ // Without having this ability, it's hard to debug the private script because syntax errors
yurys 2014/10/03 15:08:05 This code is shared between regular exception hand
aandrey 2014/10/06 10:52:57 Acknowledged.
+ // in the private script are not reported to console (the private script just crashes silently).
+ // Allowing error events in private scripts is safe because error events don't propagate to
+ // other isolated worlds (which means that the error events won't fire any event listeners
+ // in user's scripts).
+ EventDispatchForbiddenScope::AllowUserAgentEvents allowUserAgentEvents;
+ scriptState->executionContext()->reportException(event.release(), scriptId, callStack, corsStatus, true);
+ } else {
+ scriptState->executionContext()->reportException(event.release(), scriptId, callStack, corsStatus, true);
+ }
+ }
+}
+
+static void promiseRejectHandlerInMainThread(v8::Handle<v8::Promise> promise, v8::Handle<v8::Value> value, v8::PromiseRejectEvent event)
+{
+ ASSERT(isMainThread());
+
+ // It's possible that promiseRejectHandlerInMainThread() is invoked while we're initializing a window.
+ // In that half-baked situation, we don't have a valid context nor a valid world,
+ // so just return immediately.
+ if (DOMWrapperWorld::windowIsBeingInitialized())
+ return;
+
+ v8::Isolate* isolate = v8::Isolate::GetCurrent();
+
+ // If called during context initialization, there will be no entered window.
+ LocalDOMWindow* enteredWindow = enteredDOMWindow(isolate);
+ if (!enteredWindow || !enteredWindow->isCurrentlyDisplayedInFrame())
+ return;
+
+ ScriptState* scriptState = ScriptState::current(isolate);
+ ScriptValue promiseValue(scriptState, promise);
+ PromiseRejectMessageMap& map = promiseRejectMessageQueue();
+ if (event == v8::kPromiseRejectWithNoHandler) {
+ ASSERT(!map.contains(promiseValue));
+ PromiseRejectMessage* data = new PromiseRejectMessage(promiseValue, ScriptValue(scriptState, value));
yurys 2014/10/03 15:08:05 new should be wrapped with adoptPtr
aandrey 2014/10/06 10:52:57 Done.
+ map.set(promiseValue, adoptPtr(data));
+ } else if (event == v8::kPromiseHandlerAddedAfterReject) {
+ PromiseRejectMessageMap::iterator it = map.find(promiseValue);
+ if (it != map.end()) {
+ map.remove(it);
+ } else {
+ // FIXME: Report revoke message to console.
+ }
+ } else {
+ ASSERT_NOT_REACHED();
+ }
+
+ if (!promiseEndOfTurnTaskScheduled && !map.isEmpty()) {
+ promiseEndOfTurnTaskScheduled = true;
+ isolate->AddCallCompletedCallback(&firePromiseRejectMessages);
+ }
+}
+
static void failedAccessCheckCallbackInMainThread(v8::Local<v8::Object> host, v8::AccessType type, v8::Local<v8::Value> data)
{
v8::Isolate* isolate = v8::Isolate::GetCurrent();
@@ -218,6 +340,7 @@ void V8Initializer::initializeMainThreadIfNeeded()
v8::V8::SetAllowCodeGenerationFromStringsCallback(codeGenerationCheckCallbackInMainThread);
isolate->SetEventLogger(timerTraceProfilerInMainThread);
+ isolate->SetPromiseRejectCallback(promiseRejectHandlerInMainThread);
ScriptProfiler::initialize();
}

Powered by Google App Engine
This is Rietveld 408576698