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

Property "tailable: true" is not working as declared for winston File Transport #1450

Closed
mikrivorot opened this issue Aug 23, 2018 · 6 comments

Comments

@mikrivorot
Copy link

mikrivorot commented Aug 23, 2018

Environment:

  • winston@3
  • node -v : v8.6.0 and v10.7.0
  • Operating System : Windows
  • Language ES6

Problem:
I am using File winston transport in this way:

const fileTransport = new transports.File({
        filename: 'log.out',
        level: 'debug',
        maxsize: 100*1024, // 10 MB
        maxFiles: 3,
        json: false,
        tailable: true
    });

const logger = winston.createLogger({
    level: 'debug',
    format: combine(formatFile),
    transports: fileTransport,
    handleExceptions: true
});

File 'log.out' is created in necessary directory, but after the maxsize reached, logging is stopped, it means no files created.
But in 'tailable' property description "If true, log files will be rolled based on maxsize and maxfiles, but in ascending order. The filename will always have the most recent log lines. The larger the appended number, the older the log file"

I think, problem is that code of creating new and renaming old log files is unreachable:
from

let fileName = `${basename}${(i - 1)}${ext}${isZipped}`;

till

fs.rename(tmppath, path.join(this.dirname, fileName), cb);

due to internal error in asyncSeries() function:

asyncSeries(tasks, () => {

const asyncSeries = require('async/series');

and this code is also unreachable:

path.join(this.dirname, `${basename}${ext}`),

(only in case "maxFiles: 1", then tasks array is empty)

Similiar Github issues:
#589
winstonjs/winston-daily-rotate-file#23 ("still doesn't work with winston@2.4.1")
#1264

@DABH
Copy link
Contributor

DABH commented Aug 27, 2018

This may have just been fixed via merging #1420. Any chance you could give this a try using master, and seeing whether this works for you now? Thanks!

@jaxer
Copy link

jaxer commented Aug 28, 2018

I can confirm that tailable is broken in 3.0.0 and fixed in master.

@DABH
Copy link
Contributor

DABH commented Aug 28, 2018

Sweet. We will be publishing a 3.x update to npm very soon!

@mikrivorot
Copy link
Author

mikrivorot commented Aug 29, 2018

Hi everyone!
Thank you for your fix!

I tested this fix and I have some results:

  • tailable parameter works in the way it is declared. It creates log files in ascending mode.
  • but during the logging some log entries are placed in wrong log files or missed at all, and I am planning to provide examples for this case and found place in code, where the error occurred.

@DABH
Copy link
Contributor

DABH commented Aug 29, 2018

That would be great. If you figure out how to fix any errors in the code, that would be especially appreciated (feel free to open a PR!). Probably best to open a new issue if you find different errors in the code. Thanks again!

@DABH
Copy link
Contributor

DABH commented Sep 4, 2018

Closing this one for now since we got v3.1.0 published!

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

3 participants