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

Error objects are discarded during logging #280

Closed
lobodpav opened this issue Jul 17, 2013 · 52 comments
Closed

Error objects are discarded during logging #280

lobodpav opened this issue Jul 17, 2013 · 52 comments
Labels

Comments

@lobodpav
Copy link

Trying to call logger this way in a callback:

function(err) {
    // err object was created by new Error('Connection to the DB failed')
    winston.info('An error happened:', err);
}

does not print out the Error object.
By walking through the common.js code I found this section in exports.log which shall print out the err.stack:

if (meta && meta instanceof Error && meta.stack) {
    meta = meta.stack;
}

However, this does not work because there is this line:

meta = options.meta !== undefined || options.meta !== null ? exports.clone(cycle.decycle(options.meta)) : null

I.e. calling the exports.clone(...... causes to return back an empty object instead of the options.meta.

@agsha
Copy link

agsha commented Jul 18, 2013

+1 This is really bad!

@jpgarcia
Copy link

+1

1 similar comment
@dmmalam
Copy link

dmmalam commented Jul 28, 2013

+1

@dalgleish
Copy link

+1 Just bit me

@joef3
Copy link

joef3 commented Aug 13, 2013

+1

In the mean time, here's a cheezy hack -- as early as you can (only needs to be done once across all modules of course):

// A workaround for a bug in winston:
// See: https://github.com/flatiron/winston/issues/280
(function patchWinston() {
    var winstonCommon = require('winston/lib/winston/common')
        , _log = winstonCommon.log;

    function errorToStack(obj) {
        var copy;

        if (obj == null || typeof obj != 'object') {
            return obj;
        }

        if (obj instanceof Error) {
            return obj.stack;
        }

        if (obj instanceof Date || obj instanceof RegExp) {
            return obj;
        }

        if (obj instanceof Array) {
            copy = [];
            for (i in obj) {
                copy[i] = errorToStack(obj[i]);
            }
            return copy;
        }
        else {
            copy = {};
            for (k in obj) {
                if (obj.hasOwnProperty(k)) {
                    copy[k] = errorToStack(obj[k]);
                }
            }
        }
    }

    winstonCommon.log = function (options) {
        if (options != null && typeof options === 'object' && typeof options.meta === 'object') {
            options.meta = errorToStack(options.meta);
        }
        return _log(options);
    }
})();

And it's possible you might have to add more instanceof checks...

@jlukanta
Copy link

jlukanta commented Oct 7, 2013

This is my workaround:

https://gist.github.com/jlukanta/6863823

@WhatFreshHellIsThis
Copy link

+1 and I echo the sentiments earlier: this is really bad and it's over 4 months old and it's pretty critical. Is this project dead?

@agsha
Copy link

agsha commented Nov 14, 2013

I think its kinda dead. I switched over to bunyan. very satisfied.

On Thu, Nov 14, 2013 at 5:33 PM, WhatFreshHellIsThis <
notifications@github.com> wrote:

+1 and I echo the sentiments earlier: this is really bad and it's over
4 months old and it's pretty critical. Is this project dead?


Reply to this email directly or view it on GitHubhttps://github.com//issues/280#issuecomment-28529748
.

@jpgarcia
Copy link

Hey @agsha I wanted to move also to bunyan a time ago but I wasn't because of json logs but I really didn't dig into too much. Do you know if is possible to log as text like winston?

thanks!

@lobodpav
Copy link
Author

Also, I would be interested if bunyan supports email transport. Currently I use winston-email module.

@agsha
Copy link

agsha commented Nov 15, 2013

@jpgarcia bunyan primarily deals with json logs. There is no text mode in bunyan. But you can always log like this:

{txt:"My log text"}

However I found the json format much more convenient after a while. Almost every object I log has to be json formatted anyway.

@lobodpav Bunyan doesnt currently support email transport AFAIK. It seems easy to write one. See bunyan streams

@lobodpav
Copy link
Author

Thanks @agsha thanks for the hint. I could reuse the winston-mail code to implement bunyan stream.
What bothers me is the absence of plaintext. No issues with json in file stream, but logging json into console is a nonsense...

@agsha
Copy link

agsha commented Nov 15, 2013

@lobodpav As for the issue of ugly json on console, I never read the json logs without the bunyan cli

It prints the output very nicely and is very readable. Internally bunyan prints a single line of json per log record and the bunyan cli formats it very nicely. Plus the cli comes packaged with some handy filtering options.

@lobodpav
Copy link
Author

@agsha I see, piping the output to bunyan CLI is a way to get the stuff readable on console. Although, typing npm start only is a bit user friendlier.

@agsha
Copy link

agsha commented Nov 15, 2013

agree. its a little painful.

On Fri, Nov 15, 2013 at 10:46 AM, Pavel Lobodinský <notifications@github.com

wrote:

@agsha https://github.com/agsha I see, piping the output to bunyan CLI
is a way to get the stuff readable on console. Although, typing npm startonly is a bit user friendlier.


Reply to this email directly or view it on GitHubhttps://github.com//issues/280#issuecomment-28578495
.

@lobodpav
Copy link
Author

Now I realised that actually I can put this script directly into package.json:

"scripts": {
    "start": "node index.js | bunyan"
}

So actually, everything should work nicely as before :)

sineer pushed a commit to sineer/winston that referenced this issue Nov 20, 2013
@lobodpav
Copy link
Author

Yesterday, I switched over to bunyan too. No more patience to wait half a year to get logger... actually logging.
I got exactly the same functionality as I had with winston, except the missing email stream, which I will implement myself once really needed.
One great thing about bunyan is that it can create a child logger and bind specific properties to it. I.e. I can create a logger child per node module and specify module name to bunyan. This way I will always know where the log message comes from.

@yzapuchlak
Copy link

+1 to fixing this issue...

And bunyan looks interesting, I'll have to take a deeper look at it. : )

GUI added a commit to NREL/api-umbrella-gatekeeper that referenced this issue Nov 29, 2013
This mainly stems from Error objects, as well as other types not always
being logged to winston if passed as an argument:

winstonjs/winston#280
winstonjs/winston#177

We're not really using some of Winston's more advanced features right
now, so log.js seems to do the trick.
@pentateu
Copy link

+1

@rogchap
Copy link

rogchap commented Feb 21, 2014

+1

@pmendelski
Copy link

+1
This (really inconvenient) problem is 8 months old.
Is winston project still alive?

@agsha
Copy link

agsha commented Mar 17, 2014

Project is not active.

On Mon, Mar 17, 2014 at 8:59 AM, pmendelski notifications@github.comwrote:

+1
This (really inconvenient) problem is 8 months old.
Is winston project still alive?

Reply to this email directly or view it on GitHubhttps://github.com//issues/280#issuecomment-37812016
.

@robinduckett
Copy link

Uh, yes it is.

On Mon, Mar 17, 2014 at 1:46 PM, agsha notifications@github.com wrote:

Project is not active.

On Mon, Mar 17, 2014 at 8:59 AM, pmendelski <notifications@github.com

wrote:

+1
This (really inconvenient) problem is 8 months old.
Is winston project still alive?

Reply to this email directly or view it on GitHub<
https://github.com/flatiron/winston/issues/280#issuecomment-37812016>

.

Reply to this email directly or view it on GitHubhttps://github.com//issues/280#issuecomment-37816387
.

@WhatFreshHellIsThis
Copy link

Robin, your idea of "active" might differ from the rest of us.

@pmendelski
Copy link

Please prove that it's still active by fixing this issue ;)

@romanostolosh-devpronet
Copy link

+1 really annoying :(

@Guuz
Copy link

Guuz commented Aug 11, 2014

So... This is the first thing I ran into when using Winston. Is this gonna be fixed/changed in the project or what is the proposed solution?

@oleduc
Copy link

oleduc commented Aug 11, 2014

@Guuz This project appears to be inactive.

@stevenzyang
Copy link

Dammit Winston! Moving to Bunyan then.

barisusakli referenced this issue in NodeBB/NodeBB Sep 10, 2014
@jakeorr
Copy link

jakeorr commented Sep 19, 2014

+1

2 similar comments
@rus0000
Copy link

rus0000 commented Sep 24, 2014

+1

@ahowardsky
Copy link

+1

@indexzero
Copy link
Member

Dug into this. Very strange behavior. TIL that all properties on Error instances are not enumerable by default. >_<

indexzero added a commit that referenced this issue Oct 7, 2014
@psychobunny
Copy link

+1 :)

@lillem4n
Copy link

lillem4n commented Dec 5, 2014

+1

@indexzero
Copy link
Member

Fix is already committed for this. Just need to ship 0.9.0

@lillem4n
Copy link

lillem4n commented Dec 6, 2014

Yup, saw that. Still need it to be shipped, hence the +1 since I'm affected by the issue. :)

@jmcbee
Copy link

jmcbee commented Dec 16, 2014

I thought this project's dead.

@WhatFreshHellIsThis
Copy link

Utterly dead.

@metasansana
Copy link

I fell back to console.log 😒
On 16 Dec 2014 19:17, "WhatFreshHellIsThis" notifications@github.com
wrote:

Utterly dead.


Reply to this email directly or view it on GitHub
#280 (comment).

@jmcbee
Copy link

jmcbee commented Dec 17, 2014

Which commit exactly added this feature? @indexzero

@indexzero
Copy link
Member

This one on the 0.9.x branch: 8fe22b1

This project is not dead. @pose, @jcrugzz and I have been triaging old issues before shipping 0.9.0, but I expect that to happen before the new year when this will land.

If you don't want to use it because of your lack of patience then don't. No one is forcing you, but the Jedi will probably be disappointed in your attitude

@lillem4n
Copy link

Mmm, the Jedi. :)

@davidporter-id-au
Copy link

This is shit

@winstonjs winstonjs locked and limited conversation to collaborators May 28, 2015
@indexzero
Copy link
Member

Locked this issue. @desertlynx your negative and unproductive comments will get you nowhere.

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

No branches or pull requests