Skip to content
This repository has been archived by the owner on Oct 15, 2020. It is now read-only.

Commit

Permalink
chakrashim: implement promise rejection callback
Browse files Browse the repository at this point in the history
* Enabled some unhandled rejection tests (but not the Symbol one due to an
  unrelated issue)
* Added a new chakracore baseline for `unhandled_promise_trace_warnings`
* Fixed `test-inspector-debug-brk-flag` which had unhandled rejections
* Skipped `test-inspector-bindings` which had unhandled rejections

PR-URL: #469
Reviewed-By: Taylor Woll <tawoll@ntdev.microsoft.com>
Reviewed-By: Jimmy Thomson <jithomso@microsoft.com>
  • Loading branch information
kfarnung committed Feb 20, 2018
1 parent a4d7cd5 commit 219bf8c
Show file tree
Hide file tree
Showing 10 changed files with 89 additions and 12 deletions.
7 changes: 5 additions & 2 deletions deps/chakrashim/include/v8.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,10 @@ class PropertyDescriptor;
}

namespace jsrt {
JsErrorCode CreateV8PropertyDescriptor(JsValueRef descriptor,
v8::PropertyDescriptor* result);
class IsolateShim;

JsErrorCode CreateV8PropertyDescriptor(JsValueRef descriptor,
v8::PropertyDescriptor* result);
}

namespace v8 {
Expand Down Expand Up @@ -377,6 +379,7 @@ class Local {
friend JsErrorCode jsrt::CreateV8PropertyDescriptor(
JsValueRef descriptor,
v8::PropertyDescriptor* result);
friend class jsrt::IsolateShim;
template <class F> friend class FunctionCallbackInfo;
template <class F> friend class MaybeLocal;
template <class F> friend class PersistentBase;
Expand Down
21 changes: 21 additions & 0 deletions deps/chakrashim/src/jsrtisolateshim.cc
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,27 @@ JsValueRef IsolateShim::GetChakraInspectorShimJsArrayBuffer() {
return chakraInspectorShimArrayBuffer;
}

void CHAKRA_CALLBACK IsolateShim::PromiseRejectionCallback(
JsValueRef promise, JsValueRef reason, bool handled, void *callbackState) {
CHAKRA_VERIFY(callbackState != nullptr);
v8::PromiseRejectCallback callback =
reinterpret_cast<v8::PromiseRejectCallback>(callbackState);

v8::PromiseRejectMessage message(
promise,
handled ? v8::kPromiseHandlerAddedAfterReject :
v8::kPromiseRejectWithNoHandler,
reason,
v8::Local<v8::StackTrace>());

callback(message);
}

void IsolateShim::SetPromiseRejectCallback(v8::PromiseRejectCallback callback) {
JsSetHostPromiseRejectionTracker(IsolateShim::PromiseRejectionCallback,
reinterpret_cast<void*>(callback));
}

/*static*/
bool IsolateShim::RunSingleStepOfReverseMoveLoop(v8::Isolate* isolate,
uint64_t* moveMode,
Expand Down
4 changes: 4 additions & 0 deletions deps/chakrashim/src/jsrtisolateshim.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,13 +161,17 @@ class IsolateShim {
return isIdleGcScheduled;
}

void SetPromiseRejectCallback(v8::PromiseRejectCallback callback);

private:
// Construction/Destruction should go thru New/Dispose
explicit IsolateShim(JsRuntimeHandle runtime);
~IsolateShim();
static v8::Isolate * ToIsolate(IsolateShim * isolate);
static void CHAKRA_CALLBACK JsContextBeforeCollectCallback(JsRef contextRef,
void *data);
static void CHAKRA_CALLBACK PromiseRejectionCallback(
JsValueRef promise, JsValueRef reason, bool handled, void *callbackState);

JsRuntimeHandle runtime;
JsPropertyIdRef symbolPropertyIdRefs[CachedSymbolPropertyIdRef::SymbolCount];
Expand Down
2 changes: 1 addition & 1 deletion deps/chakrashim/src/v8isolate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ Local<Context> Isolate::GetCurrentContext() {
}

void Isolate::SetPromiseRejectCallback(PromiseRejectCallback callback) {
// CHAKRA does not support this explicit callback
jsrt::IsolateShim::FromIsolate(this)->SetPromiseRejectCallback(callback);
}

void Isolate::SetPromiseHook(PromiseHook hook) {
Expand Down
1 change: 0 additions & 1 deletion test/message/message.status
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,3 @@ esm_display_syntax_error : SKIP
esm_display_syntax_error_import : SKIP
esm_display_syntax_error_import_module : SKIP
esm_display_syntax_error_module : SKIP
unhandled_promise_trace_warnings : SKIP
45 changes: 45 additions & 0 deletions test/message/unhandled_promise_trace_warnings.chakracore.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
(node:*) UnhandledPromiseRejectionWarning: Error: This was rejected
at * (*test*message*unhandled_promise_trace_warnings.js:*)
at *
at *
at *
at *
at *
at *
at *
at *
at *
at *
at *
at *
at *
at *
at *
(node:*) Error: This was rejected
at * (*test*message*unhandled_promise_trace_warnings.js:*)
at *
at *
at *
at *
at *
at *
at *
at *
(node:*) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
at *
at *
at *
at *
at *
at *
at *
at *
(node:*) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 1)
at *
at handledRejection (internal/process/promises.js:*)
at Promise.prototype.then (native code:0:0)
at Promise.prototype.catch (native code:0:0)
at Anonymous function (*test*message*unhandled_promise_trace_warnings.js:*)
at *
at *
at *
2 changes: 0 additions & 2 deletions test/parallel/parallel.status
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,7 @@ test-performance-function : SKIP
test-performance-gc : SKIP
test-process-env-symbols : SKIP
test-promise-internal-creation : SKIP
test-promises-unhandled-rejections : SKIP
test-promises-unhandled-symbol-rejections : SKIP
test-promises-warning-on-unhandled-rejection : SKIP
test-regress-GH-12371 : SKIP
test-repl : SKIP
test-repl-inspector : SKIP
Expand Down
1 change: 1 addition & 0 deletions test/sequential/sequential.status
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ test-inspector-async-call-stack-abort : SKIP
test-inspector-async-hook-setup-at-inspect-brk : SKIP
test-inspector-async-stack-traces-promise-then : SKIP
test-inspector-async-stack-traces-set-interval : SKIP
test-inspector-bindings : SKIP
test-inspector-break-when-eval : SKIP
test-inspector-contexts : SKIP
test-inspector-scriptparsed-context : SKIP
Expand Down
18 changes: 12 additions & 6 deletions test/sequential/test-inspector-debug-brk-flag.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,20 @@ async function testBreakpointOnStart(session) {
{ 'method': 'Debugger.setPauseOnExceptions',
'params': { 'state': 'none' } },
{ 'method': 'Debugger.setAsyncCallStackDepth',
'params': { 'maxDepth': 0 } },
{ 'method': 'Profiler.enable' },
{ 'method': 'Profiler.setSamplingInterval',
'params': { 'interval': 100 } },
'params': { 'maxDepth': 0 } }
];

if (process.jsEngine !== 'chakracore') {
commands.push(
{ 'method': 'Profiler.enable' },
{ 'method': 'Profiler.setSamplingInterval',
'params': { 'interval': 100 } });
}

commands.push(
{ 'method': 'Debugger.setBlackboxPatterns',
'params': { 'patterns': [] } },
{ 'method': 'Runtime.runIfWaitingForDebugger' }
];
{ 'method': 'Runtime.runIfWaitingForDebugger' });

session.send(commands);
await session.waitForBreakOnLine(0, session.scriptPath());
Expand Down

0 comments on commit 219bf8c

Please sign in to comment.