winstonjs / winston-daily-rotate-file

A transport for winston which logs to a rotating file each day.
MIT License
889 stars 151 forks source link

auditFile is not relative to dirname #346

Open brandonryan opened 2 years ago

brandonryan commented 2 years ago

When you specify the auditFile option, its relative to cwd instead of the dirname.

wbt commented 2 years ago

Can you please include a link to any documentation that would lead you to expect otherwise?

brandonryan commented 2 years ago

Given that all other file options are relative to dirname, it is inconsistent compared to them. If this is intentional the documentation should probably state that explicitly. Or maybe it should be a file name instead of a path like the others.

wbt commented 2 years ago

It looks like it was documented as a file name instead of a path; this commit added that. However, it does look like that second insertion should have been AFTER path.join(self.dirname, instead of before. I'm also curious why this line uses self.dirname while the filename a few lines up refers to this.dirname.

wbt commented 2 years ago

@mattberther Might you be able to comment on if that was a bug or intentional?

mattberther commented 2 years ago

@wbt winston-daily-rotate-file uses rogerc/file-stream-rotator under the hood and passes auditFile through. It appears as though the file-stream-rotator resolves a path, if one is not present (see https://github.com/rogerc/file-stream-rotator/blob/2b41e6a9a6447394999a4ea03a695d8398238969/FileStreamRotator.js#L220).

mattberther commented 2 years ago

@wbt self.dirname vs this.dirname was unintentional, but shouldnt impact this issue.

mlb-6300 commented 2 years ago

Any updates on this? I think when specifying an audit file name when creating a new transport it should be going off dirname and not cwd. If this is deliberate, it should definitely be documented in the description for the audit file option here (https://github.com/winstonjs/winston-daily-rotate-file#options)