Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exposing a new JSRT method to get additional information about exceptions #2936

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions bin/NativeTests/JsRTApiTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,9 @@ namespace JsRTApiTest
{
bool value;
JsValueRef exception = JS_INVALID_REFERENCE;
JsValueRef exceptionMetadata = JS_INVALID_REFERENCE;
JsValueRef metadataValue = JS_INVALID_REFERENCE;
JsPropertyIdRef property = JS_INVALID_REFERENCE;
JsValueType type;

REQUIRE(JsHasException(&value) == JsNoError);
Expand All @@ -782,6 +785,84 @@ namespace JsRTApiTest
REQUIRE(JsSetException(exception) == JsNoError);
REQUIRE(JsHasException(&value) == JsNoError);
CHECK(value == true);

REQUIRE(JsGetAndClearExceptionWithMetadata(&exceptionMetadata) == JsNoError);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be nice to verify this API in the case where there is no exception thrown?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, which required some fixes in the JSRT functions.

REQUIRE(JsHasException(&value) == JsNoError);
CHECK(value == false);

REQUIRE(JsGetPropertyIdFromName(_u("exception"), &property) == JsNoError);
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
CHECK(metadataValue == exception);

REQUIRE(JsGetPropertyIdFromName(_u("line"), &property) == JsNoError);
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
CHECK(type == JsNumber);

REQUIRE(JsGetPropertyIdFromName(_u("column"), &property) == JsNoError);
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
CHECK(type == JsNumber);

REQUIRE(JsGetPropertyIdFromName(_u("length"), &property) == JsNoError);
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
CHECK(type == JsNumber);

REQUIRE(JsGetPropertyIdFromName(_u("url"), &property) == JsNoError);
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
CHECK(type == JsString);

REQUIRE(JsGetPropertyIdFromName(_u("source"), &property) == JsNoError);
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
CHECK(type == JsString);


REQUIRE(JsHasException(&value) == JsNoError);
CHECK(value == false);
REQUIRE(JsGetAndClearExceptionWithMetadata(&exceptionMetadata) == JsErrorInvalidArgument);
CHECK(exceptionMetadata == JS_INVALID_REFERENCE);


REQUIRE(JsRunScript(_u("@ bad syntax"), JS_SOURCE_CONTEXT_NONE, _u(""), nullptr) == JsErrorScriptCompile);
REQUIRE(JsHasException(&value) == JsNoError);
CHECK(value == true);

REQUIRE(JsGetAndClearExceptionWithMetadata(&exceptionMetadata) == JsNoError);
REQUIRE(JsHasException(&value) == JsNoError);
CHECK(value == false);

REQUIRE(JsGetPropertyIdFromName(_u("exception"), &property) == JsNoError);
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
CHECK(type == JsError);

REQUIRE(JsGetPropertyIdFromName(_u("line"), &property) == JsNoError);
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
CHECK(type == JsNumber);

REQUIRE(JsGetPropertyIdFromName(_u("column"), &property) == JsNoError);
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
CHECK(type == JsNumber);

REQUIRE(JsGetPropertyIdFromName(_u("length"), &property) == JsNoError);
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
CHECK(type == JsNumber);

REQUIRE(JsGetPropertyIdFromName(_u("url"), &property) == JsNoError);
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
CHECK(type == JsString);

REQUIRE(JsGetPropertyIdFromName(_u("source"), &property) == JsNoError);
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
CHECK(type == JsString);
}

TEST_CASE("ApiTest_ExceptionHandlingTest", "[ApiTest]")
Expand Down
33 changes: 33 additions & 0 deletions lib/Jsrt/ChakraCore.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,39 @@ JsGetModuleHostInfo(
_Outptr_result_maybenull_ void** hostInfo);

#ifdef CHAKRACOREBUILD_
/// <summary>
/// Returns metadata relating to the exception that caused the runtime of the current context
/// to be in the exception state and resets the exception state for that runtime. The metadata
/// includes a reference to the exception itself.
/// </summary>
/// <remarks>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you lay out the returned metadata object in <remarks>? I've always hated that I've no clue what's in exception of JsGetAndClearException without looking into source code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

/// <para>
/// If the runtime of the current context is not in an exception state, this API will return
/// <c>JsErrorInvalidArgument</c>. If the runtime is disabled, this will return an exception
/// indicating that the script was terminated, but it will not clear the exception (the
/// exception will be cleared if the runtime is re-enabled using
/// <c>JsEnableRuntimeExecution</c>).
/// </para>
/// <para>
/// The metadata value is a javascript object with the following properties: <c>exception</c>, the
/// thrown exception object; <c>line</c>, the 0 indexed line number where the exception was thrown;
/// <c>column</c>, the 0 indexed column number where the exception was thrown; <c>length</c>, the
/// source-length of the cause of the exception; <c>source</c>, a string containing the line of
/// source code where the exception was thrown; and <c>url</c>, a string containing the name of
/// the script file containing the code that threw the exception.
/// </para>
/// <para>
/// Requires an active script context.
/// </para>
/// </remarks>
/// <param name="metadata">The exception metadata for the runtime of the current context.</param>
/// <returns>
/// The code <c>JsNoError</c> if the operation succeeded, a failure code otherwise.
/// </returns>
CHAKRA_API
JsGetAndClearExceptionWithMetadata(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JsGetAndClearExceptionWithMetadata [](start = 0, length = 34)

@liminzhu Should this be in ChakraCommon.h instead of core only?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment this is still experimental and not yet officially supported, which is why I put it in ChakraCore.h

_Out_ JsValueRef *metadata);

/// <summary>
/// Called by the runtime to load the source code of the serialized script.
/// </summary>
Expand Down
100 changes: 96 additions & 4 deletions lib/Jsrt/Jsrt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "ByteCode/ByteCodeSerializer.h"
#include "Common/ByteSwap.h"
#include "Library/DataView.h"
#include "Library/JavascriptExceptionMetadata.h"
#include "Library/JavascriptSymbol.h"
#include "Library/JavascriptPromise.h"
#include "Base/ThreadContextTlsEntry.h"
Expand Down Expand Up @@ -855,15 +856,15 @@ CHAKRA_API JsSetContextData(_In_ JsContextRef context, _In_ void *data)
END_JSRT_NO_EXCEPTION
}

void HandleScriptCompileError(Js::ScriptContext * scriptContext, CompileScriptException * se)
void HandleScriptCompileError(Js::ScriptContext * scriptContext, CompileScriptException * se, const WCHAR * sourceUrl)
{
HRESULT hr = se->ei.scode;
if (hr == E_OUTOFMEMORY || hr == VBSERR_OutOfMemory || hr == VBSERR_OutOfStack || hr == ERRnoMemory)
{
Js::Throw::OutOfMemory();
}

Js::JavascriptError* error = Js::JavascriptError::CreateFromCompileScriptException(scriptContext, se);
Js::JavascriptError* error = Js::JavascriptError::CreateFromCompileScriptException(scriptContext, se, sourceUrl);

Js::JavascriptExceptionObject * exceptionObject = RecyclerNew(scriptContext->GetRecycler(),
Js::JavascriptExceptionObject, error, scriptContext, nullptr);
Expand Down Expand Up @@ -2544,7 +2545,10 @@ CHAKRA_API JsGetAndClearException(_Out_ JsValueRef *exception)
Js::JavascriptExceptionObject *recordedException = nullptr;

BEGIN_TRANSLATE_OOM_TO_HRESULT
recordedException = scriptContext->GetAndClearRecordedException();
if (scriptContext->HasRecordedException())
{
recordedException = scriptContext->GetAndClearRecordedException();
}
END_TRANSLATE_OOM_TO_HRESULT(hr)

if (hr == E_OUTOFMEMORY)
Expand Down Expand Up @@ -2999,7 +3003,7 @@ JsErrorCode RunScriptCore(JsValueRef scriptSource, const byte *script, size_t cb
{
PERFORM_JSRT_TTD_RECORD_ACTION_NOT_IMPLEMENTED(scriptContext);

HandleScriptCompileError(scriptContext, &se);
HandleScriptCompileError(scriptContext, &se, sourceUrl);
return JsErrorScriptCompile;
}

Expand Down Expand Up @@ -4590,4 +4594,92 @@ CHAKRA_API JsGetWeakReferenceValue(
});
}

CHAKRA_API JsGetAndClearExceptionWithMetadata(_Out_ JsValueRef *metadata)
{
PARAM_NOT_NULL(metadata);
*metadata = nullptr;

JsrtContext *currentContext = JsrtContext::GetCurrent();

if (currentContext == nullptr)
{
return JsErrorNoCurrentContext;
}

Js::ScriptContext *scriptContext = currentContext->GetScriptContext();
Assert(scriptContext != nullptr);

if (scriptContext->GetRecycler() && scriptContext->GetRecycler()->IsHeapEnumInProgress())
{
return JsErrorHeapEnumInProgress;
}
else if (scriptContext->GetThreadContext()->IsInThreadServiceCallback())
{
return JsErrorInThreadServiceCallback;
}

if (scriptContext->GetThreadContext()->IsExecutionDisabled())
{
return JsErrorInDisabledState;
}

HRESULT hr = S_OK;
Js::JavascriptExceptionObject *recordedException = nullptr;

BEGIN_TRANSLATE_OOM_TO_HRESULT
if (scriptContext->HasRecordedException())
{
recordedException = scriptContext->GetAndClearRecordedException();
}
END_TRANSLATE_OOM_TO_HRESULT(hr)

if (hr == E_OUTOFMEMORY)
{
recordedException = scriptContext->GetThreadContext()->GetRecordedException();
}
if (recordedException == nullptr)
{
return JsErrorInvalidArgument;
}

Js::Var exception = recordedException->GetThrownObject(nullptr);

if (exception == nullptr)
{
// TODO: How does this early bailout impact TTD?
return JsErrorInvalidArgument;
}

return ContextAPIWrapper<false>([&](Js::ScriptContext* scriptContext, TTDRecorder& _actionEntryPopper) -> JsErrorCode {
Js::Var exceptionMetadata = Js::JavascriptExceptionMetadata::CreateMetadataVar(scriptContext);
Js::JavascriptOperators::OP_SetProperty(exceptionMetadata, Js::PropertyIds::exception, exception, scriptContext);

Js::FunctionBody *functionBody = recordedException->GetFunctionBody();
if (functionBody == nullptr)
{
// This is probably a parse error. We can get the error location metadata from the thrown object.
Js::JavascriptExceptionMetadata::PopulateMetadataFromCompileException(exceptionMetadata, exception, scriptContext);
}
else
{
if (!Js::JavascriptExceptionMetadata::PopulateMetadataFromException(exceptionMetadata, recordedException, scriptContext))
{
return JsErrorInvalidArgument;
}
}

*metadata = exceptionMetadata;

#if ENABLE_TTD
if (hr != E_OUTOFMEMORY)
{
PERFORM_JSRT_TTD_RECORD_ACTION(scriptContext, RecordJsRTGetAndClearExceptionWithMetadata);
PERFORM_JSRT_TTD_RECORD_ACTION_RESULT(scriptContext, metadata);
}
#endif


return JsNoError;
});
}
#endif // CHAKRACOREBUILD_
1 change: 1 addition & 0 deletions lib/Jsrt/JsrtCommonExports.inc
Original file line number Diff line number Diff line change
Expand Up @@ -118,4 +118,5 @@
JsCreatePromise
JsCreateWeakReference
JsGetWeakReferenceValue
JsGetAndClearExceptionWithMetadata
#endif
2 changes: 1 addition & 1 deletion lib/Jsrt/JsrtInternal.h
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ JsErrorCode SetContextAPIWrapper(JsrtContext* newContext, Fn fn)
return errorCode;
}

void HandleScriptCompileError(Js::ScriptContext * scriptContext, CompileScriptException * se);
void HandleScriptCompileError(Js::ScriptContext * scriptContext, CompileScriptException * se, const WCHAR * sourceUrl = nullptr);

#if DBG
#define _PREPARE_RETURN_NO_EXCEPTION __debugCheckNoException.hasException = false;
Expand Down
5 changes: 5 additions & 0 deletions lib/Runtime/Base/JnDirectFields.h
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ ENTRY(customSections)
ENTRY(initial)
ENTRY(maximum)
ENTRY(element)
ENTRY(line)
// End Wasm

// SIMD_JS
Expand Down Expand Up @@ -729,6 +730,10 @@ ENTRY(isLockFree)
ENTRY(wait)
ENTRY(wake)

ENTRY(column)
ENTRY(url)
ENTRY(exception)

// Note: Do not add fields for conditionally-compiled PropertyIds into this file.
// The bytecode for internal javascript libraries is built on chk but re-used in fre builds.
// Having a mismatch in the number of PropertyIds will cause a failure loading bytecode.
Expand Down
53 changes: 52 additions & 1 deletion lib/Runtime/Debug/TTActionEvents.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
//-------------------------------------------------------------------------------------------------------
#include "RuntimeDebugPch.h"

#include "Library/JavascriptExceptionMetadata.h"

#if ENABLE_TTD

namespace TTD
Expand Down Expand Up @@ -390,6 +392,52 @@ namespace TTD
throw TTDebuggerAbortException::CreateAbortEndOfLog(_u("End of log reached with Host Process Exit -- returning to top-level."));
}

void GetAndClearExceptionWithMetadataAction_Execute(const EventLogEntry* evt, ThreadContextTTD* executeContext)
{
TTD_REPLAY_ACTIVE_CONTEXT(executeContext);

HRESULT hr = S_OK;
Js::JavascriptExceptionObject *recordedException = nullptr;

BEGIN_TRANSLATE_OOM_TO_HRESULT
if (ctx->HasRecordedException())
{
recordedException = ctx->GetAndClearRecordedException();
}
END_TRANSLATE_OOM_TO_HRESULT(hr)

Js::Var exception = nullptr;
if (recordedException != nullptr)
{
exception = recordedException->GetThrownObject(nullptr);
}

if (exception != nullptr)
{
Js::ScriptContext * scriptContext = executeContext->GetActiveScriptContext();
Js::Var exceptionMetadata = Js::JavascriptExceptionMetadata::CreateMetadataVar(scriptContext);

Js::FunctionBody *functionBody = recordedException->GetFunctionBody();

Js::JavascriptOperators::OP_SetProperty(exceptionMetadata, Js::PropertyIds::exception, exception, scriptContext);

if (functionBody == nullptr)
{
// This is probably a parse error. We can get the error location metadata from the thrown object.
Js::JavascriptExceptionMetadata::PopulateMetadataFromCompileException(exceptionMetadata, exception, scriptContext);
}
else
{
if (!Js::JavascriptExceptionMetadata::PopulateMetadataFromException(exceptionMetadata, recordedException, scriptContext))
{
return;
}
}

JsRTActionHandleResultForReplay<JsRTResultOnlyAction, EventKind::GetAndClearExceptionWithMetadataActionTag>(executeContext, evt, exceptionMetadata);
}
}

void GetAndClearExceptionAction_Execute(const EventLogEntry* evt, ThreadContextTTD* executeContext)
{
TTD_REPLAY_ACTIVE_CONTEXT(executeContext);
Expand All @@ -398,7 +446,10 @@ namespace TTD
Js::JavascriptExceptionObject *recordedException = nullptr;

BEGIN_TRANSLATE_OOM_TO_HRESULT
recordedException = ctx->GetAndClearRecordedException();
if (ctx->HasRecordedException())
{
recordedException = ctx->GetAndClearRecordedException();
}
END_TRANSLATE_OOM_TO_HRESULT(hr)

Js::Var exception = nullptr;
Expand Down
1 change: 1 addition & 0 deletions lib/Runtime/Debug/TTActionEvents.h
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,7 @@ namespace TTD
void AllocateFunctionAction_Execute(const EventLogEntry* evt, ThreadContextTTD* executeContext);

void HostProcessExitAction_Execute(const EventLogEntry* evt, ThreadContextTTD* executeContext);
void GetAndClearExceptionWithMetadataAction_Execute(const EventLogEntry* evt, ThreadContextTTD* executeContext);
void GetAndClearExceptionAction_Execute(const EventLogEntry* evt, ThreadContextTTD* executeContext);
void SetExceptionAction_Execute(const EventLogEntry* evt, ThreadContextTTD* executeContext);

Expand Down
Loading