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

JSON String interpolation not working as expected. #745

Closed
Nepoxx opened this issue Nov 4, 2015 · 13 comments
Closed

JSON String interpolation not working as expected. #745

Nepoxx opened this issue Nov 4, 2015 · 13 comments
Labels

Comments

@Nepoxx
Copy link

Nepoxx commented Nov 4, 2015

The following code:

let logger = require('winston')

logger.log('info', 'test %j', {a: 5, b: 10})
logger.info('test %j', {a: 5, b: 10})

Results in:

info: test %j a=5, b=10
info: test %j a=5, b=10

This is not as expected and described here.

@indexzero
Copy link
Member

Potentially fixed by #737. Could you try to reproduce with that fork and let me know?

@indexzero indexzero added the Bug label Nov 5, 2015
@Nepoxx
Copy link
Author

Nepoxx commented Nov 5, 2015

Tested pr/737, works as expected:

info: test {"a":5,"b":10}
info: test {"a":5,"b":10}

@indexzero
Copy link
Member

@Nepoxx will try to get this fixed on Friday during our winston maintenance. But #737 breaks a number of tests, so will need to find another approach.

@jsumners
Copy link
Contributor

Sooooo, how's this coming along?

@indexzero
Copy link
Member

@jsumners due to a variety of factors dev work has been slowed the last couple of weeks. Hoping to change that in a few weeks.

@alexandrubau
Copy link

Hi guys, any news regarding this fix?
I think it's the same with #774.

Thanks

@sandrocsimas
Copy link

+1

@alexandrubau
Copy link

The PR #777 seems to be a fix.

@indexzero
Copy link
Member

@alexandrubau I think this is unrelated to #777. The bug itself appears to be in the lack of check of length in the interpolation matches vs. the number of arguments defined. See the code: https://github.com/winstonjs/winston/blob/master/lib/winston/logger.js#L170

jsumners added a commit to jsumners/winston that referenced this issue Mar 23, 2016
@jsumners jsumners mentioned this issue Mar 23, 2016
@jsumners
Copy link
Contributor

I submitted a potential fix for this a few days ago. Has anyone had a chance to review it?

@thomasdelaet
Copy link

+1

@thomasdelaet
Copy link

Hey,

I think I fixed this while developing this wrapper

@indexzero
Copy link
Member

Fixed by #835

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

No branches or pull requests

6 participants