feat(inspector): implement console in the runtime. fix console logs in devtools inspector (#884) #894

petekanev · 2017-12-14T13:41:42Z

Prerequisite - PR #893

Fixes Console.log broken in Chrome DevTools #884 Console logs not displaying properly in the Console panel inside Chrome Devtools of Chrome 62 and later versions.
Fixes NativeScript+angular stack traces/errors not displaying context (module name, line number, etc) nativescript-cli#3158 - console.log sources are not shown, making debugging of external libraries more difficult for users.
As a result of the fix above the scripts now display the source where the console methods were invoked (right hand-side). A timestamp of when they've been logged is also displayed.

Another major change that occurred as part of the fix was refactoring of the console.js script, which previously resided in the tns-core-modules - which is now implemented in its own Console module as part of the NativeScript Android Runtime implementation - this eliminates the need for special checks, if-clauses and weird workarounds needed in the tns-core-modules code base to make script work in all cases.

The implementation of log, warn, info, error is such that they can now print a list of consecutive values, while the current implementation could only print 1 value at a time per call.
Currently a normally valid call to console.log() such as console.log(5, 12, "redundantString", { a: "Pesho", age: 22 }); would fail.
The new implementation would print 5 12 "redundantString" { a: "Pesho", age: 22 }, similar to what a nodeJS, or browser application would print out.
As a result of the refactoring the strings passed as arguments to console.log() and the likes will no longer support c-string-like format strings (%d, %s, etc.) interpolated with values from the argument list following the primary arguments to the log(..) call. - Users whose logs depended on that feature should migrate to using JavaScript template literals (backtick strings) - https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals
As part of the implementation, if an object is console.log()ed , the new implementation will JSONStringify the object now, instead of transforming it into [object Object]
console.time() and console.timeEnd() will work without passing them any arguments, default will be the identifier used in that case.
Introduce non-standard behavior - logging an object which contains circular references will print out as Couldn't convert object to a JSON string: Uncaught TypeError: Converting circular structure to JSON
E.g.

function Foo() {
    this.abc = "Hello";
     this.circular = this;
}
var foo = new Foo();
console.log(foo);
console.dir(foo);

Outputs:

Couldn't convert object to a JSON string: Uncaught TypeError: Converting circular structure to JSON // console.log(foo)

==== object dump start ==== // console.dir(foo)
abc: "Hello"
circular: #CR
==== object dump end ====

Tested with Chrome DevTools, and the VSCode extension in the most basic scenarios.

ns-bot · 2017-12-14T14:06:52Z

💚

Plamen5kov · 2017-12-18T14:04:56Z

test-app/runtime/src/main/cpp/console/Console.cpp

+        return;
+    }
+
+    auto stack = v8::StackTrace::CurrentStackTrace(isolate, 1, v8::StackTrace::StackTraceOptions::kDetailed);


what's the "1" for?

"1"'s for how many stackframes of the stacktrace I'd like to retrieve, it's in the v8 StackTrace API. This is the approach to acquiring the current frame, in order to read the executing script's name, and the line where the call occurred.

Plamen5kov · 2017-12-18T14:07:20Z

test-app/runtime/src/main/cpp/console/Console.cpp

+    std::stringstream ss;
+
+    auto argLen = info.Length();
+    if (argLen) {


suggestion: if no arguments are passed maybe we can print a new line, since that's the common behavior

Nice catch! That will align the behavior with how node's console.log() with empty args does.

ns-bot · 2017-12-18T14:29:51Z

💔

Plamen5kov · 2017-12-18T14:38:04Z

test-app/runtime/src/main/cpp/console/Console.cpp

+    auto expressionPasses = argLen && info[0]->BooleanValue();
+
+    if (!expressionPasses) {
+        std::stringstream ss;


rename to assertionError or something

the expressionPasses is the first argument which is the test, and determines whether to print an error or not.

Plamen5kov · 2017-12-18T14:48:54Z

test-app/runtime/src/main/cpp/console/Console.cpp

+            auto propertiesLen = propNames->Length();
+            for (int i = 0; i < propertiesLen; i++) {
+                auto propName = propNames->Get(context, i).ToLocalChecked();
+                auto property = argObject->Get(context, propName).ToLocalChecked();


rename to propertyValue

Plamen5kov · 2017-12-18T14:50:07Z

test-app/runtime/src/main/cpp/console/Console.cpp

+            ss << "==== object dump end ====" << std::endl;
+        } else {
+            v8::Local<v8::String> argString;
+            info[0]->ToString(isolate->GetCurrentContext()).ToLocal(&argString);


consider buildstring logic

Plamen5kov · 2017-12-18T14:57:06Z

test-app/runtime/src/main/cpp/console/Console.cpp

+    return ss.str();
+}
+
+void Console::traceCallback(const v8::FunctionCallbackInfo<v8::Value>& info) {


run:

debugger; console.trace(",smth");

on root level in app

Plamen5kov · 2017-12-18T15:10:53Z

test-app/runtime/src/main/cpp/v8_inspector/src/inspector/v8-log-agent-impl.h

+        static void EntryAdded(const std::string& text, std::string verbosityLevel, std::string url, int lineNumber);
+
+        static V8LogAgentImpl* Instance;
+        protocol::Log::Frontend m_frontend;


consider moving the fields to the private fields

Plamen5kov

put messages in emptry methods that contain ErrorStrings* parameter
fix console.trace() problem

ns-bot · 2017-12-18T15:30:49Z

💔

ns-bot · 2017-12-19T11:54:35Z

💔

petekanev · 2017-12-19T12:15:05Z

@Plamen5kov please review the changes

ns-bot · 2017-12-19T12:17:15Z

💔

Plamen5kov

great work, LGTM

petekanev · 2017-12-19T13:21:18Z

run ci

ns-bot · 2017-12-19T13:25:36Z

💔

Natalia-Hristova · 2017-12-19T13:44:13Z

run ci

ns-bot · 2017-12-19T13:50:15Z

💔

ns-bot · 2017-12-19T15:57:19Z

💔

petekanev · 2017-12-19T16:04:16Z

run ci

ns-bot · 2017-12-19T16:15:31Z

💔

ns-bot · 2017-12-19T17:18:22Z

💔

petekanev · 2017-12-20T08:30:06Z

run ci

ns-bot · 2017-12-20T09:01:19Z

💔

petekanev · 2017-12-20T09:04:45Z

I found an error where the JSON.stringify inside the object transformation would crash the application if it tries to stringify an object with a circular reference. Will fix it before merging

Remove the console.ts/js implementation from the tns-core-modules and instead bootstrap the console prototype with log, warn, dir, trace, time, timeEnd, info, assert callbacks.

… assertion fails

…erties

… references JSON.stringify fails to resolve

ns-bot · 2017-12-20T13:00:21Z

💚

ns-bot · 2017-12-20T13:09:39Z

💚

ns-bot · 2017-12-20T13:19:34Z

💚

…e logs in devtools inspector (#884) (#894)" This reverts commit 6480aae.

…e logs in devtools inspector (#884) (#894)" (#920) This reverts commit 6480aae.

…x console logs in devtools inspector (#884) (#894)"" This reverts commit 5bcfbde.

petekanev added the feature label Dec 14, 2017

petekanev self-assigned this Dec 14, 2017

petekanev requested review from vakrilov and Plamen5kov December 14, 2017 13:41

petekanev added this to the 4.0.0 (TBD) milestone Dec 14, 2017

petekanev changed the title ~~Fix console logs in devtools inspector (#884). Implement console in the runtime~~ (feat):Implement console in the runtime. fix console logs in devtools inspector (#884) Dec 14, 2017

petekanev changed the title ~~(feat):Implement console in the runtime. fix console logs in devtools inspector (#884)~~ feat(inspector): implement console in the runtime. fix console logs in devtools inspector (#884) Dec 15, 2017

petekanev mentioned this pull request Dec 15, 2017

NativeScript+angular stack traces/errors not displaying context (module name, line number, etc) NativeScript/nativescript-cli#3158

Closed

Plamen5kov reviewed Dec 18, 2017

View reviewed changes

petekanev force-pushed the pete/fix-console-logs branch from b2b307e to 5d3f41f Compare December 18, 2017 14:25

Plamen5kov reviewed Dec 18, 2017

View reviewed changes

NativeScript deleted a comment from petekanev Dec 18, 2017

Plamen5kov reviewed Dec 18, 2017

View reviewed changes

Plamen5kov suggested changes Dec 18, 2017

View reviewed changes

petekanev force-pushed the pete/fix-console-logs branch from dd675c8 to 9eb9b7d Compare December 19, 2017 11:50

Plamen5kov approved these changes Dec 19, 2017

View reviewed changes

petekanev force-pushed the pete/fix-console-logs branch from 557e0f7 to 8666e66 Compare December 19, 2017 15:31

petekanev added 7 commits December 20, 2017 14:34

update the __consoleMessage callback by implementing devtools log agent

318fa4d

add c++ utf16 string to protocol utf16 string conversion method

8b4c973

feat: implement the console API in the runtime

0f52b71

Remove the console.ts/js implementation from the tns-core-modules and instead bootstrap the console prototype with log, warn, dir, trace, time, timeEnd, info, assert callbacks.

chore: remove references to __console global callback

17d42bf

fix: encoding problem with utf16 strings used in console

e8df179

refactor console.assert to use console.log string building logic when…

9da881b

… assertion fails

fix console.dir() implementation to display the values of object prop…

8780bad

…erties

petekanev force-pushed the pete/fix-console-logs branch from d32097e to ee00593 Compare December 20, 2017 12:34

fix console logging implementation to print out a message on circular…

b3bc525

… references JSON.stringify fails to resolve

petekanev force-pushed the pete/fix-console-logs branch from ee00593 to b3bc525 Compare December 20, 2017 12:45

petekanev mentioned this pull request Dec 20, 2017

fix console.log tests conforming to the new format NativeScript/functional-tests#9

Merged

petekanev merged commit 6480aae into master Jan 2, 2018

petekanev deleted the pete/fix-console-logs branch January 2, 2018 14:13

petekanev added a commit that referenced this pull request Jan 8, 2018

Revert "feat(inspector): implement console in the runtime. fix consol…

5bcfbde

…e logs in devtools inspector (#884) (#894)" This reverts commit 6480aae.

petekanev mentioned this pull request Jan 8, 2018

Revert "feat(inspector): implement console in the runtime. fix consol… #920

Merged

petekanev added a commit that referenced this pull request Jan 9, 2018

Revert "feat(inspector): implement console in the runtime. fix consol…

b6da591

…e logs in devtools inspector (#884) (#894)" (#920) This reverts commit 6480aae.

petekanev added a commit that referenced this pull request Jan 9, 2018

Revert "Revert "feat(inspector): implement console in the runtime. fi…

0af5697

…x console logs in devtools inspector (#884) (#894)"" This reverts commit 5bcfbde.

petekanev mentioned this pull request Jan 26, 2018

Revert "revert console" #933

Merged

petekanev restored the pete/fix-console-logs branch February 6, 2018 11:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(inspector): implement console in the runtime. fix console logs in devtools inspector (#884) #894

feat(inspector): implement console in the runtime. fix console logs in devtools inspector (#884) #894

petekanev commented Dec 14, 2017 •

edited

Loading

ns-bot commented Dec 14, 2017

Plamen5kov Dec 18, 2017

petekanev Dec 18, 2017

Plamen5kov Dec 18, 2017

petekanev Dec 18, 2017

ns-bot commented Dec 18, 2017

Plamen5kov Dec 18, 2017

petekanev Dec 18, 2017

Plamen5kov Dec 18, 2017

Plamen5kov Dec 18, 2017

Plamen5kov Dec 18, 2017

Plamen5kov Dec 18, 2017

Plamen5kov left a comment

ns-bot commented Dec 18, 2017

ns-bot commented Dec 19, 2017

petekanev commented Dec 19, 2017

ns-bot commented Dec 19, 2017

Plamen5kov left a comment

petekanev commented Dec 19, 2017

ns-bot commented Dec 19, 2017

Natalia-Hristova commented Dec 19, 2017

ns-bot commented Dec 19, 2017

ns-bot commented Dec 19, 2017

petekanev commented Dec 19, 2017

ns-bot commented Dec 19, 2017

ns-bot commented Dec 19, 2017

petekanev commented Dec 20, 2017

ns-bot commented Dec 20, 2017

petekanev commented Dec 20, 2017

ns-bot commented Dec 20, 2017

ns-bot commented Dec 20, 2017

ns-bot commented Dec 20, 2017

feat(inspector): implement console in the runtime. fix console logs in devtools inspector (#884) #894

feat(inspector): implement console in the runtime. fix console logs in devtools inspector (#884) #894

Conversation

petekanev commented Dec 14, 2017 • edited Loading

ns-bot commented Dec 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ns-bot commented Dec 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Plamen5kov left a comment

Choose a reason for hiding this comment

ns-bot commented Dec 18, 2017

ns-bot commented Dec 19, 2017

petekanev commented Dec 19, 2017

ns-bot commented Dec 19, 2017

Plamen5kov left a comment

Choose a reason for hiding this comment

petekanev commented Dec 19, 2017

ns-bot commented Dec 19, 2017

Natalia-Hristova commented Dec 19, 2017

ns-bot commented Dec 19, 2017

ns-bot commented Dec 19, 2017

petekanev commented Dec 19, 2017

ns-bot commented Dec 19, 2017

ns-bot commented Dec 19, 2017

petekanev commented Dec 20, 2017

ns-bot commented Dec 20, 2017

petekanev commented Dec 20, 2017

ns-bot commented Dec 20, 2017

ns-bot commented Dec 20, 2017

ns-bot commented Dec 20, 2017

petekanev commented Dec 14, 2017 •

edited

Loading