Skip to content

dateFile error handling fix#1097

Merged
lamweili merged 2 commits intolog4js-node:masterfrom
4eb0da:datefile-error-handling
Jan 19, 2022
Merged

dateFile error handling fix#1097
lamweili merged 2 commits intolog4js-node:masterfrom
4eb0da:datefile-error-handling

Conversation

@4eb0da
Copy link

@4eb0da 4eb0da commented Oct 4, 2021

Make the same that "file" appender does

@lamweili lamweili added this to the 6.4.0 milestone Jan 18, 2022
Copy link
Contributor

@lamweili lamweili left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not so sure if we should implement the alive to prevent further writing.
Perhaps should keep to the current behavior for the time being.

EDIT: Should be fine since the stream is closed anyway. The alive might save some processing (such as removing color).

The stream is closed when the 'error' event is emitted unless the autoDestroy option was set to false when creating the stream.
(src: https://nodejs.org/api/stream.html#event-error)

@lamweili lamweili merged commit e86a809 into log4js-node:master Jan 19, 2022
@lamweili
Copy link
Contributor

lamweili commented Jan 19, 2022

If an EventEmitter does not have at least one listener registered for the 'error' event, and an 'error' event is emitted, the error is thrown, a stack trace is printed, and the Node.js process exits.
(src: https://nodejs.org/docs/latest/api/events.html#error-events)

Similar to #529 and #1089, the dateFileAppender causes the Node.js process to exit as there is no listener for its 'error' event. This PR by @4eb0da added the listener to prevent such crashes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

If save the log as a file, a disk failure will cause the node to crash.

2 participants