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

File/Line numbers? #21

Closed
sam-lex opened this issue Sep 16, 2017 · 23 comments
Closed

File/Line numbers? #21

sam-lex opened this issue Sep 16, 2017 · 23 comments

Comments

@sam-lex
Copy link
Contributor

sam-lex commented Sep 16, 2017

Currently the line numbers are lost: showing as index.js which points to where the console.log is being called
screen shot 2017-09-16 at 3 16 58 pm

This picture shows as vendor.bundle.js because I had --sourcemap=false on.

I did managed to get it to show the "correct" file/line like so:
screen shot 2017-09-16 at 3 32 55 pm

But with a major downside, and that is having to make the call with extra ().

// before
this.logger.trace('Hi there');
// after
this.logger.trace('Hi there')();
// changes
this.logger.trace('Hi there') // returns a "formatted console.log function"
// so executing right after would result into showing the right file/line number

To avoid breaking the api, we could set a flag to make it either log directly or return a function.
I can't think of a "pretty" way of doing this though. Any thoughts?

@sam-lex
Copy link
Contributor Author

sam-lex commented Sep 16, 2017

have a look, I'll make a pull request if this is ok:
sam-lex@0f1a832

Changes:

  • added logImmediately option, defaults to true
  • _log now returns a formatted console log function

Usage:

// Same as before, no changes
this.logger.trace('Hi there'); // Hi there                   index.js ...

// or .. set 'logImmediately' to false:
LoggerModule.forRoot({
  serverLogLevel: NgxLoggerLevel.TRACE,
  level: NgxLoggerLevel.TRACE,
  logImmediately: false
}),
// and add '()' when using
this.logger.trace('Hi there')(); // Hi there                app.component.ts ...
// if 'logImmediately' is not defined or set to true.
// It will log twice. Once inside the logger function, and once on the 'caller' side.

_logIE(..) part is not implemented yet. For now, it just returns an empty function () => {} at the end.

Not sure if it's worth the trouble though, adding '()' can be quite annoying .. and ugly.

@avatsaev
Copy link

so, was this merged?

@sam-lex
Copy link
Contributor Author

sam-lex commented Sep 19, 2017

nope, not yet ...
I've been using this "new way" of adding extra (). Works ok, but many times I log and forget the extra (), so I go and test my app and keep thinking why my log doesn't get called, then I keep looking at different places where I might have messed up, until I realized I forgot the ().

So instead of really helping, it could hinder your development (sometimes).

Tried looking at how other loggers do it, they don't (without some sort of performance implications - like using the Error().stack and parsing it somehow).

@avatsaev
Copy link

yep I just tried by packaging your fork, it works but it feels hacky with the extra ().

Without showing the file and line number it's pretty difficult to debug.

@dbfannin
Copy link
Owner

@sam-lex, I'm not sure I like returning a function for this non logImmediately function. However, I can see the benefit in being able to have the line number from where the logger was called.

Here are my thoughts:

  1. Most users are probably writing their log messages so that they are unique and can be easily found using the logged message alone. This is a pretty standard best practice for logging.

  2. Mixing the intent of the logging methods doesn't follow the SOLID practices.

So how do we extend the logger with these things in mind:

  1. We can't penalize the majority of the users
  2. We need to be explicit in how the logger works without obscurity (when possible)
  3. We should do our best to be "SOLID".

One idea would be to have a function that builds and returns the arguments that are passed to console.log. You could use the logger to build out the arguments, but call console.log in your own file:

const args = this.logger.getArgs(NgxLoggerLevel.ERROR, 'My error message', {more: 'stuff here'});
console.log(...args)

getArgs would return

[
    `%c2017-09-19T13:00:56Z [Error] %cMy error message`, 
    `color:red`, 
    `color:black`, 
    {more: 'stuff here'}
]

I realize this would be more code to type than adding () at the end of the logger call, but this new method could be leveraged by the existing logging methods and it would have a single purpose. Also, when using it this way, you wouldn't accidentally forget to add the (), but instead it would be very clear what arguments the method needs to complete its task.

Your thoughts?

@sam-lex
Copy link
Contributor Author

sam-lex commented Sep 19, 2017

Yeah, adding () is really not ideal. Creating a getArgs function to later call it yourself with console.log does make it clear and easy to understand what's happening, but I think it's way too much code to make it worth.

Since my javascript knowledge is very limited, I'll have to research more before I can suggest another solution.

One idea that I got, but have no clue how to do it, or if it's even possible:

Override the console.log method. (yes this sounds terrible, but hold on..)
Just to make it call the _logOnServer method.
then, call the original console.log.

Reason, it's possible to return a "formatted" console.log.
With slight modifications we could change:
debug(...args) to debug = (formatted console log function)
so we could use it like so:
this.logger.debug('hi there');

which is perfect except that it won't log to the server.

So if it's possible to somehow chain the _logOnServer to console.log automatically, or override it at instance level so that it won't affect the normal console, then we can get this done beautifully.

@leo6104
Copy link
Contributor

leo6104 commented Sep 19, 2017

I found some stack overflow documents (related to stack trace / caller function tracking)

https://stackoverflow.com/questions/280389/how-do-you-find-out-the-caller-function-in-javascript

We can trace caller function by arguments.callee.caller.caller in private _logIE function or private _log function

https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Global_Objects/Function/caller

There is some method to find out the implement file name & position from function reference.

https://stackoverflow.com/questions/1340872/how-to-get-javascript-caller-function-line-number-how-to-get-javascript-caller
Cross browser support stack trace -> https://github.com/stacktracejs/stacktrace.js

To support this method properly with all browser, it might be require some additional dependency.
If we accept additional dependency, this feature can be supported with above method. :)

@dbfannin
Copy link
Owner

@sam-lex, I don't think we should go the route of overriding console.log. It would never be a good idea to do something like this. Since console is shared globally, it would not just make the change for your code, but also for every other module that you've imported. It could have some pretty gnarly side effects.

@leo6104, It appears that arguments.callee.caller is deprecated.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/arguments/callee

@leo6104
Copy link
Contributor

leo6104 commented Sep 19, 2017

@dbfannin 😭 Okay, i try to find another way. Thank you for giving updated information. (Korean references are not up-to-date translations.)

@Tom4U
Copy link

Tom4U commented Oct 31, 2017

Any news on this? Seems to be some kind of common problem in JS as I could find out through Google search.

@sam-lex
Copy link
Contributor Author

sam-lex commented Nov 1, 2017

not on my side, I've been using my own work-around version, but is not really useful to me anymore, it's taking a long time to build my angular project. angular/angular-cli#5775 (comment)

so with ng serve --sourcemap=false, is just shows main.bundle.js: ...line number

@abielikesu
Copy link

Hi Guys,

@dbfannin your idea about the args is great, and it actually has many more use cases. It resembles my favourite GoLang logging library: https://github.com/Sirupsen/logrus

The syntax I normally use goes like this:
log.WithFields(log.Fields{ "module": "Animals", "animal": "walrus", }).Info("A walrus appears")

@a-stangl
Copy link

I'm currently testing some logging libraries and loglevel was the only one that solves the "File/Line numbers" problem. Maybe it can be used as an inspiration ;)

@dbfannin
Copy link
Owner

@stan187, the library loglevel has a slightly different use case. They do not format or modify the console text at all, and they also don't support logging on serverside. Because of this, they can bind their methods to console. Unfortunately, we can't do that.

However, since we're still having a lot of pull for this feature, I've made some changes so that we can log the filename and line number along with the logs. After #56, this feature should be supported!

@sellmic
Copy link

sellmic commented Apr 12, 2018

loglevel has a plugin for server side logging
https://github.com/kutuluk/loglevel-plugin-remote

@dbfannin
Copy link
Owner

filename:line number is now supported in [email protected]. Hopefully this helps!

@JavierFuentes
Copy link

JavierFuentes commented May 20, 2018

Hi @dbfannin , I just upgraded one project from Angular CLI 1.7.4 with ngx-logger: ^2.2.4 to Angular CLI 6.0.3 with ngx-logger ^3.0.0-rc1 and filename:linenumber do now reference to main.js all the time when i run ng serve from command line...

Example:
2018-05-20T18:36:33.722Z TRACE [main.js:4703] mergeFields ngx-logger.js:386

@zhaodong-wang
Copy link

@JavierFuentes have you solved this main.js issue?

@dbfannin
Copy link
Owner

@TeXnicians follow #69 for updates.

@ghost
Copy link

ghost commented May 26, 2019

For me the line numbers work if I add --eval-source-map e.g.:

ng serve --host 0.0.0.0 --ssl --port 4200 --eval-source-map

@Zenguro
Copy link

Zenguro commented May 29, 2019

For me the line numbers work if I add --eval-source-map e.g.:

ng serve --host 0.0.0.0 --ssl --port 4200 --eval-source-map

Doesn't seem to work in the latest Angular version 7.2.x

@ghost
Copy link

ghost commented May 29, 2019

@Zenguro I am on 7.2.2 - must be something else. But as one would expect, --eval-source-map is set to be deprecated and as usual no alternative is neither documented nor announced. Classic.

@ghost
Copy link

ghost commented Jan 19, 2020

Back again. Now using Angular 8. The option is still deprecated but ng serve --eval-source-map does too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests