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

fix(lib): File Transports Not Emitting Errors On Stream Errors #581

Closed
wants to merge 5 commits into from

Conversation

callbacknull
Copy link
Contributor

In this commit file & daily-rotate-file add an 'error' event listener on
their stream so that we can emit those errors from the transport.

fixes #511

In this commit file & daily-rotate-file add an 'error' event listener on
their stream so that we can emit those errors from the transport.

fixes winstonjs#511
@indexzero
Copy link
Member

@ambbell we should attempt to reopen the file stream before emitting the error. Also, this is a breaking change so it will go into the next major.

Alex Bell added 2 commits March 18, 2015 15:17
This commit only deals with streams created by the transport.

Changes:
- Adds public property `maxRetries` -> defaults 2
- Adds internal property `failures` -> # of failed stream creation attempts
- Transport will attempt to re-create the stream on errors
- Will attempt to re-create while `failures < maxRetries`
This commit adds a conditional check to file / daily-rotate-file tranports
logging function. We check to make that the transport isn't currently in a
failed state (i.e failures >= maxRetries) if it is in a failed state then
we perform a NOOP and return an error to the logging callback.
@callbacknull
Copy link
Contributor Author

@indexzero great idea - how does this look? If it looks good I'll add a couple of unit tests and push those as well.

@indexzero
Copy link
Member

Overall lgtm now! We are getting pretty close to starting work on 1.0.0, but it will be a while before it is actually released.

Alex Bell added 2 commits March 19, 2015 12:07
changes:
- 2 tests added under file-test & daily-rotate-file-test
- 1 helper function added that is named `assertFailedTransport`
@indexzero
Copy link
Member

Thanks! Cherry-picked and will go out in winston@1.0.0 which will be released shortly.

@indexzero indexzero closed this Apr 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

logger error event handler not working
2 participants