rogerc / file-stream-rotator Goto Github PK

Hi @philmeehan

Let's see how we can keep maintaining this package or at least suggest alternative drop-in replacements for it, because there are projects relying on it which require attention.

I'll be happy to help with this

Hello, file-stream-rotator team. It seems that there are some breaking changes in this module. My package.json includes

"winston": "^3.0.0-rc2",
"winston-daily-rotate-file": "^3.1.4"

My app fails to run in recent builds due to TypeError: Cannot read property '1' of null. How does this come out? Should I remove all the old logs before?

TypeError: Cannot read property '1' of null
    at Object.FileStreamRotator.getStream (/opt/nodeapp/node_modules/file-stream-rotator/FileStreamRotator.js:396:38)
    at new DailyRotateFile (/opt/nodeapp/node_modules/winston-daily-rotate-file/daily-rotate-file.js:81:57)

relates to https://github.com/rogerc/file-stream-rotator/blob/master/FileStreamRotator.js#L396

The following code gives me a log file that has one line constantly appended to and no new lines.

I believe this is due to file-stream-rotator as it provides the stream where the file is written to, correct?

var FileStreamRotator = require('file-stream-rotator');
var httpLogger = require('morgan');
var logDirectory = path.join(__dirname, 'logs');
var path = require('path');

// Creates a rotating write stream
var accessLogStream = FileStreamRotator.getStream({
  date_format: 'YYYY-MM-DD',
  filename: path.join(logDirectory, '%DATE%_serverJs.log'),
  frequency: 'daily',
  verbose: true
});

app.use(httpLogger('combined', {stream: accessLogStream}));

Hi,

I have a situation where I need my files to have an extension(filename.ext). When I configure the file name as follow,

fileStreamRotator.getStream({ frequency: "daily", filename:"filename-%DATE%.ext" });

on file rotation the file name is suffixed with a counter as follow filename-DATE.ext.1 , filename-DATE.ext.2 and so on, but actually my requirement is filename-DATE-1.ext, filename-DATE-2.ext. I need the extension to be suffixed at last, instead of the counter.

Something like this will do great.

fileStreamRotator.getStream({ frequency: "daily", filename:"filename-%DATE%", extension: "ext" });

I mean newest file name is "a.log", and older files are 'a.log.2019_09_22', 'a.log.2019_0921'. A fixed file name is convenient for Kibana to collect

I'm trying to update https://github.com/winstonjs/winston-daily-rotate-file to use [email protected] and am encountering some issues. In an attempt to isolate the issue, I came up with the following script which eliminates the winston transport and demonstrates the issue using the file-stream-rotator directly.

var os = require('os');

var logStream = require('file-stream-rotator').getStream({
    filename: './logs/application-%DATE%.log',
    frequency: 'custom',
    size: '1k',
    max_logs: 4,
    end_stream: true
});

for (var i = 0; i < 10; i++) {
    logStream.write(randomString(1056) + os.eol);
}

The above code eventually errors with something similar to Error: ENOENT: no such file or directory, watch 'logs/application-201909032108.log.9'. Other times, the error says something like: Error: EEXIST: file already exists, symlink './logs/application-201909032111.log.2' -> './logs/current.log'.

In the event that I don't receive any errors, the script just hangs until I manually break it with CTRL-C.

Please let me know what else you need to help troubleshoot the issue.

It would be really handy if logs could be compressed on the fly or zipped automatically on rotation.

winston-daily-rotate-file calls such an option zippedArchive: A boolean to define whether or not to gzip archived log files. (default 'false')

They seem to compress the logfile in one piece on rotation:

        if (options.zippedArchive) {
            this.logStream.on('rotate', function (oldFile) {
                var gzip = zlib.createGzip();
                var inp = fs.createReadStream(oldFile);
                var out = fs.createWriteStream(oldFile + '.gz');
                inp.pipe(gzip).pipe(out).on('finish', function () {
                    fs.unlinkSync(oldFile);
                });
            });
        }

I guess it would be too risky to use on-the-fly compression because in case of an abrupt process termination the file could be rendered invalid...

The behavior of file-stream-rotator varies depending on when it starts up. Note the following code snippet from FileStreamRotator.getStream

var dateFormat = (options.date_format || DATE_FORMAT);
    if(frequencyMetaData && frequencyMetaData.type == "daily"){
        if(!options.date_format){
            dateFormat = "YYYY-MM-DD";
        }
        if(moment().format(dateFormat) != moment().add(2,"hours").format(dateFormat) || moment().format(dateFormat) == moment().add(1,"day").format(dateFormat)){
            if(options.verbose){
                console.log(new Date(),"[FileStreamRotator] Changing type to custom as date format changes more often than once a day or not every day");
            }
            frequencyMetaData.type = "custom";
        }
    }

The value of moment() is dependent on the time of day getStream is run. Therefore, comparing moment() with moment().add(2,"hours") to discover if you are on the same day or not will result in different behavior depending on whether moment() is evaluated at eg 11:30pm or 11:30am.

Please let me know if you would accept an MR to fix this issue.

Hi. Your project depends from old version of moment. Can you update it to latest? One of the problem old version is global moment definition. It was been deprecated later, see changelog

Specifying daily as the frequency is completely broken. It will create a new logfile every minute.

The culprit is here, which triggers every minute because getDate always returns the current minute for daily rotation.

As I find the code very hard to read and understand and the test coverage evidently isn't sufficient, I've decided not to make a pull request - this would be likely to break some other piece of functionality.

Most logs do not grow fast enough, by my estimate (wrong?), to be rotated "daily". A "monthly" option would be nice to have. I could try to use "Xm" and calculate (X as 30_24_60) but that assumes all months have 30 days.

I can submit a PR if there is interest.

rotator attempts to delete the file that does not exist.

Below my log.
2018-05-10T11:46:16.430Z '[FileStreamRotator] Could not remove old log file: ' 'log/exception_2018-04-07.log'

Yes, today is May 10.
The file has already been deleted.
The Message show start node.js and midnight and raises an UnException Socket Error.
Always First Rotator's Error and Next Socket Error.
There was no problem before adding Rotator.

How do I resolve this issue?

Hello Roger,
I am trying to have no date appended in current/active log, for that options I gave are:
{ level: process.env.LOG_LEVEL,
filename: ${appRoot}/logs/mylog,
handleExceptions: true,
json: true,
maxSize: '100m', // ~100MB
maxFiles: 15,
colorize: false,
tailable: false,
frequency: 'd',
extension: '.log',
}

but after size exceeds the new file created is mylog.log.1 but still logging continues in mylog.log,
I guess on line 581 var newLogfile = logfileName; can be given , not sure though :)

Please let me know if I am selecting correct options

The things printed to the console are only debugging messages. They should be removed or only enabled with a debug flag.

• file-stream-rotator: v 0.2.1
• node: v8.9.0
• max_logs: 7d

When a lot of user access and continuously generate logs, this audit array will become pretty large, and it will exit in memory, what's more this array has been used in filter, it will seriously affect server performance.

CVE-2017-18214. The moment module before 2.19.3 for Node.js is prone to a regular expression denial of service via a crafted date string, a different vulnerability than CVE-2016-4055.

we are using PM2 , morgan with file stream-rotator in production environment.we have given maxsize 50m. but it is rotating when size is 300mb. we have set the size parameter as 50m.
file-stream-rotator is having issue with PM2?

I'd like to write to a syslog the name of the purged file. I think a good approach would be to have this event available.

Hi,

It is not working correctly if the file name contains a directory.

const path = require('path');

const directory = "dir";
const name = "file.ext";
const nameWithNumber = "file.ext.2";

const fullPath = path.join(directory, name);
const FullNameWithNumber =path.join(directory, nameWithNumber);

console.log(FullNameWithNumber.match(fullPath)) /// null
console.log(nameWithNumber.match(name)) /// value

• It seems at the moment the rotator file is using the MD5 hash option for the crypto function but to turn this FIPS compliant it would be a simple change to turn the crypto function to use the new FIPS complaint hash of SHA256
• If you look here https://github.com/rogerc/file-stream-rotator/blob/master/FileStreamRotator.js#L278 the change would be to change to SHA256
• This would make other applications which use file-stream-rotator to allow be FIPS validated, increasing the security and popularity of this package and any package which utilizes this such as https://github.com/winstonjs/winston-daily-rotate-file for example.

If this makes sense a PR for this should not be too difficult. You can even parametrize the function to allow for FIPS enablement or not for those packages that don't want a SHA256 encryption.

whether it supports user-defined series number filename format like applicaiton_001.log, not application.log.1

It should only keep the last X items, where X should have a sane default and should be configurable by the user.

fs.createWriteStream will create a new file, but if it's in a directory that doesn't exist, you're going to have a bad day.

Looks like @rogerc has taken ownership of the project so the README needs a bit of a tidy up

(I was contacted directly by email enquiring about taking it over)

I would like to have an option to disable a creating symbolic link on a "current.log" file.
Why this is forced?

Thank you!

Hi @rogerc @mattberther,

I'm trying to clean up some of my dependencies for a project and noticed this repo uses moment. Can I submit a PR to replace that dependency with a more lightweight alternative like dayjs? I just replaced moment with dayjs in my own project and liked it because it's much smaller but has a similar API, although I'm sure there're plenty of other similar libraries that could be used.

Thanks!

please update https://nodesecurity.io/advisories/55
@danjenkins
@rogerc
@viktort

We received the following error on our test service, causing it to crash. We don't know what caused it either. All I can say is that it's on a portable hdd.

node:events:356
      throw er; // Unhandled 'error' event
      ^

Error: UNKNOWN: unknown error, write
Emitted 'error' event at:
    at WriteStream.<anonymous> (E:\Projects\Node\Stew2\node_modules\file-stream-rotator\FileStreamRotator.js:677:15

I also issued this to winston-daily-rotate-file, which is dependent on this project.

WE have a requirement where we are using winston-daily-rotate-file and internally it is using this project. However all the log files (even the current one) is being created with the DATE appended to it (that means the file name is not constant). This is creating problems for the log aggregator that takes the data from the current log file.

we need to provide an option where we can do it. I have done some changes in this and it seems to be working ok after the changes, so just wanted to know if we can submit a PR here. once it is done here, we can pass extra options via WINSTON-DAILY-ROTATE-FILE and it will all ok.

>> node --version
v14.17.6
>> npm --version
7.24.1

>> sw_vers
ProductName:    macOS
ProductVersion: 11.6.2
BuildVersion:   20G314

"file-stream-rotator": "^0.5.7"

Creates symlink properly

new transports.DailyRotateFile({
    filename: `${filePath}-%DATE%.log`,
    datePattern: 'YYYY-MM-DD',
    zippedArchive: true,
    maxFiles: '14d',
    createSymlink: true,
    symlinkName: 'test.log',
});

Fails to create a symlink. Just a regular file with the symlinkName is created

const symlinkFileName = 'test.log';
new transports.DailyRotateFile({
    filename: `${filePath}-%DATE%.log`,
    datePattern: 'YYYY-MM-DD',
    zippedArchive: true,
    maxFiles: '14d',
    createSymlink: true,
    symlinkName: symlinkFileName
});

My project run well when I print node app.js
But When I set 'filename' option I can't browse my page
then I remove this option. the page is running well,but log file not generate...

There are scenarios where the FileStreamRotator will crash due to the following line:

Since the filename comparison uses a dynamic RegEx there are scenarios where the .match will crash due to unescaped characters in the path.

For example consider a path where the ( characters exists which is an opening of a capture group. If no closing exists, the script will crash with an syntax error regarding not closing a capture group.

const lastEntry = "some/path";
const tlog = "some/other/path(";

lastEntry.match(tlog); // Uncaught SyntaxError: Invalid regular expression: /some/other/path(/: Unterminated group

Is there a reason why this has to use .match? Wouldn't it suffice with something like:

lastEntry.includes(tlog);

// or even

lastEntry === tlog;

Let me know what you think and I'll create a PR for it :)

Thanks
BR

https://codeclimate.com/github/holidayextras/file-stream-rotator/issues

Hi

i am looking for option to change the file based on file size.. if possible please let me know.

Regards
keshav

Please, use readFileSync and JSON.parse instead of require for audit files here:

This causes issues with bundlers like Webpack, since they try to be smart about non-constant require statements, and start complaining when they can't narrow them down to a reasonable degree. To suppress the complaints, I currently have to use the following Webpack plugin:

new webpack.ContextReplacementPlugin(/file-stream-rotator$/, ctx => {
  for (const x of ctx.dependencies) {
    x.critical = false;
  }
})

This wouldn't be necessary with readFileSync (which the current usage of require is a shortcut for).

I created a simple express program that does nothing just to see if the rotate event is being fired but unfortunately it does not.
Example: (Rotate every minute)
package version: 0.5.5

Neither of us have commit access to the repo any more

Error: EACCES: permission denied, mkdir '/opt/iobroker/'
  File "fs.js", line 934, col 3, in Object.mkdirSync
  File "/opt/iobroker/node_modules/file-stream-rotator/FileStreamRotator.js", line 654, col 20, in null.<anonymous>
  ?, in Array.reduce
  File "/opt/iobroker/node_modules/file-stream-rotator/FileStreamRotator.js", line 642, col 27, in mkDirForFile
  File "/opt/iobroker/node_modules/file-stream-rotator/FileStreamRotator.js", line 609, col 17, in Object.<anonymous>
  File "/opt/iobroker/node_modules/winston-daily-rotate-file/daily-rotate-file.js", line 148, col 20, in DailyRotateFile.log
  File "/opt/iobroker/node_modules/winston-transport/index.js", line 102, col 17, in DailyRotateFile._write
  File "/opt/iobroker/node_modules/readable-stream/lib/_stream_writable.js", line 428, col 64, in doWrite
  File "/opt/iobroker/node_modules/readable-stream/lib/_stream_writable.js", line 417, col 5, in writeOrBuffer
  File "/opt/iobroker/node_modules/readable-stream/lib/_stream_writable.js", line 334, col 11, in DailyRotateFile.Writable.write

In fact there is no try/catch in the method and it also should not be needed, but it seems that "existsSync" is returning false also when file exists but is not accessable?!

There are some circumstances that log file may be removed while the stream is writing to it. It would be great if the logic to check if file no longer exists added, and create (rotate) a new file (of the same name) should the file was removed.

414 if (newDate != curDate || (fileSize && curSize > fileSize) || !fs.existsSync( logfile ) )

@rogerc I'm the maintainer of https://github.com/winstonjs/winston-daily-rotate-file. We've recently transitioned that project to using your file-stream-rotator component. As such, I've submitted some PRs against this project to incorporate changes that would be beneficial to our use cases.

We have some other things that we would like to do - things that probably do not fall under your project's charter. As such, I was opening this issue to request your permission to take a copy of your FileStreamRotator.js and include it directly in the winston-daily-rotate-file module and evolve it from there.

Thank you in advance for considering my request.

Environment :

OS : SUSE Linux Enterprise Server 15 SP2
Node: 14.16.0
Express: 4.17.1,
winston: 3.2.1,
winston-daily-rotate-file: 4.4.2

I am using following code :

 var transport = new (winston.transports.DailyRotateFile)({
        filename: 'log/server-%DATE%.log',
        datePattern: 'YYYY-MM-DD-HH',
        maxSize: '100m', //100MB
        zippedArchive: true,
        maxFiles: '10',
        frequency: '24h'
    });

and getting error as "Digest method not supported" from FileStreamRotator ( which usage crypto submodule of nodejs)

Below is actual line which was throwing error from File "FileStreamRotator.js" under node_modules :
crypto.createHash('md5').update(logfile + "LOG_FILE" + time).digest("hex")

I posted similar question on winston-daily-rotate-file repo (winstonjs/winston-daily-rotate-file#340) , but as this issue seems related to FileStreamRotator and dependent crypto submodule of node, posting the question here.

Also raised same question on stack-overflow : https://stackoverflow.com/questions/71552510/error-digest-method-not-supported-while-using-winston-transports-dailyrotate

Please check below screenshot for openssl version and algorithm list for the platforms where I am using the function ( Please note : for Helios platform function is working fine without any error but for SLES platform it does throw error )

Can someone please help to identify the issue here and probable solution?

Since MIT license explicitly requires the inclusion of the license text, it is insufficient to merely state the license used as "MIT" in package.json.

Quote from the MIT license from https://mit-license.org/

... The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

Solution: create license.md or license.txt to include the content of the MIT license.

I have multiple streams, boiled down to an example:

var stream1 = fsr.getStream({
  date_format: 'YYYY-MM-DD-HH-mm-ss',
  filename: logDir + '/stream1.%DATE%',
  frequency: 'custom',
  verbose: false,
  max_logs: 2,
  audit_file: logDir + '/.audit1.json'
});
var stream2 = fsr.getStream({
  date_format: 'YYYY-MM-DD-HH-mm-ss',
  filename: logDir + '/stream2.%DATE%',
  frequency: 'custom',
  verbose: false,
  max_logs: 2,
  audit_file: logDir + '/.audit2.json'
});

app.use((req, res, next) => {
  stream1.write('x\n');
  stream2.write('x\n');
  next();
})

Initially I had it without the "audit_file" but they were both sharing the .audit.json and seemed to not each be getting their full "max_logs" allotment. So then I tried adding separate audit_file locations, but it had no effect it only create one.

Is this supposed to work with multiple streams in the same application? In my case I'd have separate streams for access logs and error logs.

When you have multiple processes writing to the same logfile and using the symlink feature we ended up in this exception sometimes:

Error: EEXIST: file already exists, symlink 'iobroker.2020-01-11.log' -> '/opt/iobroker/log/iobroker.current.log'
at Object.symlinkSync (fs.js:909:3)
at createCurrentSymLink (/opt/iobroker/node_modules/file-stream-rotator/FileStreamRotator.js:303:16)
at EventEmitter. (/opt/iobroker/node_modules/file-stream-rotator/FileStreamRotator.js:530:17)
at EventEmitter.emit (events.js:203:15)
at /opt/iobroker/node_modules/file-stream-rotator/FileStreamRotator.js:607:20
at process._tickCallback (internal/process/next_tick.js:61:11)
at Function.Module.runMain (internal/modules/cjs/loader.js:834:11)
at startup (internal/bootstrap/node.js:283:19)
at bootstrapNodeJSCore (internal/bootstrap/node.js:623:3)

From my Analysis the following happens:
1.) When multiple processes do their first write at "0:0:0" (in our case because daily rotate) then we execute https://github.com/rogerc/file-stream-rotator/blob/master/FileStreamRotator.js#L296-L299 partially multiple times and nearly each of these commands can produce an ENOENT
2.) When this happens multiple processes execute the problematic line and the first one will succeed, all the others will get an EEXISTS exception

Solution:
1.) In https://github.com/rogerc/file-stream-rotator/blob/master/FileStreamRotator.js#L303 there is no try/catch for the command and so it throws the uncatchable exception. So at least there should be a try/catch added
2.) This try catch normally should never happen, so question is what should be done when it happens?

• ignore and assume such a case - risk is that something different happened and we end up in an "not current" symlink
• check for EEXISTS and check the new symlink location if it already matched to the correct one ( such a check could be added in general also on top because if link is already correct nothing is needed to be done.
• other ideas?!

I would create a PR if you would be open to an option for writing times in UTC. I've just had issues in the past with logs in varying timezones and it was a hassle to manage.

It had been some time since last commit. I want to fix the issues/44. But the repo hadn't consistent ode-style.

Is it all right to add prettier (config and devDeps) to make the code looks consistent?

If the file-stream-rotator writes a record to the log and exceeds the maxSize size requirements informed in the transport for Winston, it will not call the rotator event and on the next run it will just create a new log

issue for winston-daily-rotate-file

Here is the verification for size file, if the size exceeds, than it will create the next log, but wont zip the old one.

        if(lastLogFile){
            var lastLogFileStats = fs.statSync(lastLogFile);
            if(lastLogFileStats.size < fileSize){
                t_log = lastLogFile;
                fileCount--;
                curSize = lastLogFileStats.size;
            } 
        }

Here is the verification for size file when is writing, it is necessary a check after writing to the file.

        stream.write = (function (str, encoding) {
            var newDate = this.getDate(frequencyMetaData, dateFormat, options.utc);
            if (newDate != curDate || (fileSize && curSize > fileSize)) {
....
                stream.emit('rotate',oldFile, newLogfile);
                BubbleEvents(rotateStream,stream);
            }
            rotateStream.write(str, encoding);
            // Handle length of double-byte characters
            curSize += Buffer.byteLength(str, encoding);

if the last written register (before the application closes) exceeds the file size to rotate, then the next time the application is run, the first block of code above will be executed and will create a new log sequence, and the old one will be left unzipped

I openned a issue for winston-daily-rotate-file about this problem: issue for winston-daily-rotate-file

Is it possible to keep logging to a file with the same name and add a timeformat to archive files