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

[Logs] console.trace calls show up as errors #37

Closed
lostintangent opened this issue Sep 16, 2016 · 11 comments
Closed

[Logs] console.trace calls show up as errors #37

lostintangent opened this issue Sep 16, 2016 · 11 comments

Comments

@lostintangent
Copy link

lostintangent commented Sep 16, 2016

This is a pretty minor issue, but since error messages display the red icon, it would be great if users could visually scan their logs for issues, without seeing any false negatives.

Repro steps:

  1. Add a call to console.trace into your app
  2. Run the app to exercise the code that calls [ClientHttp] Error when selecting Web Services tab with large data sets #1
  3. Open up the Glimpse client and select the respective request in the list
  4. Select the Logs tab

Expected: To see the call to console.trace, displayed as an informational message
Actual: The trace call shows up, but it is labeled as an error.

capture

@nikmd23 nikmd23 self-assigned this Sep 16, 2016
@nikmd23
Copy link
Member

nikmd23 commented Sep 16, 2016

@lostintangent, I'm a bit confused as to what the issue you're reporting is.

Is it that this message isn't actually an "Error" but it being reported as such?

@lostintangent
Copy link
Author

lostintangent commented Sep 16, 2016

That screenshot is showing the result of me calling console.trace within an app. The message is the stack that was generated as a result, but Glimpse seems to be categorizing the log as an error, which it isn't. Apologies for the ambiguity, I just added repro steps to the bug to hopefully make this clearer.

@nikmd23
Copy link
Member

nikmd23 commented Sep 21, 2016

Oh! I see, okay. Thanks for the clarification.

@mike-kaufman
Copy link
Contributor

See https://github.com/nodejs/node/blob/master/lib/console.js#L82_L90. Console.trace(...) writes output to through Console.error(...). IIRC, we're not proxying trace.

@nikmd23
Copy link
Member

nikmd23 commented Sep 22, 2016

Okay, it looks like the issue here is a difference in the implementation of the/a spec.

The DevTools Working Group says that trace is "equivalent to console.log except that it also adds stack trace from the point where the method was called". This is in line with @lostintangent's expectation. However, Node's documentation states that calls to .trace should be written to stderr. (That's true for both 4.5 LTS and 6.6.)

I do find Node's implementation to be a bit funny, but we should probably stick to their definition until we have a bit more user feedback.

That said, calls to console.trace() in the browser should have the behavior that @lostintangent described. We need to test to make sure this is the case. @newtonjain, please make sure this case is covered in your related work.

@mike-kaufman
Copy link
Contributor

We can fairly easily proxy trace for node such that it still writes to stderr, but the glimpse message has an info or a warning. This is a small, low-risk change.

@nikmd23
Copy link
Member

nikmd23 commented Sep 26, 2016

I like @mike-kaufman's suggestion, and think that we should make sure the Glimpse message has an Info Debug tag on it.

@nikmd23
Copy link
Member

nikmd23 commented Nov 14, 2016

console.trace() calls from Node are still showing up as Errors. They should be marked as Debug.

@nikmd23 nikmd23 modified the milestones: M015, M014 Nov 14, 2016
@nikmd23 nikmd23 assigned mike-kaufman and unassigned avanderhoorn Nov 14, 2016
@mike-kaufman
Copy link
Contributor

Addressed with PR https://github.com/Glimpse/Glimpse.Node/pull/633.

@nikmd23
Copy link
Member

nikmd23 commented Nov 18, 2016

👌

I did find a related issue while testing this (https://github.com/Glimpse/Glimpse.Browser.Agent/issues/86), but this looks good.

@nikmd23
Copy link
Member

nikmd23 commented Nov 28, 2016

This is now available in Glimpse for Node 0.15.2. Please see our announcement issue for more information.

@nikmd23 nikmd23 closed this as completed Nov 28, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants