search

iccicci / rotating-file-stream

Opens a stream.Writable to a file rotated by interval and/or size. A logrotate alternative.
MIT License
286 stars 39 forks source link

Sometimes crashes on interval change #95

Closed diwic closed 1 year ago

diwic commented 1 year ago

Hi,

I'm experiencing occasional crashes, maybe once a week from an application that logs a lot. The message is: TypeError: Cannot read properties of undefined (reading write) at RotatingFileStream.rewrite (<myapp>\node_modules\rotating-file-stream\dist\cjs\index.js:89:33)

These are my log settings:

interval="1h"
maxSize="18G"
size="50M"
immutable=true
teeToStdout=true

I notice that the crashes are always on the hour. Trying to look into the issue myself, I have a theory that this could happen if there is more than one chunk to write and set() (the function started inside interval()) runs between chunk writes.

I e,

1) task A: rewrite, awaits this.file.write 2) task B: interval set, destroys this.file and sets this.timeoutPromise 3) task A: await finishes and goes on to write the next chunk, but this.file is now destroyed so the app crashes.

The obvious fix would be to move await timeoutPromise to inside the chunk loop in rewrite().

I haven't actually tested this because it takes weeks to figure out if it actually worked, but it seems likely to me. What do you think?

iccicci commented 1 year ago

Hi @diwic ,

thank you for the nice report! I'll check this in the next days.

iccicci commented 1 year ago

Hi @diwic ,

I taken a short look and it was enough to realize you are right! Nice catch! If you want to open a simlpe pull request with the fix, I'll merge it soon.

Thank you

diwic commented 1 year ago

@iccicci Thanks for the library and quick review :-)