fix: Create a new console logger for each snapshot #407
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This will be reverted soon. Just so I can test on a fork without publishing anything (or dealing with symlinks).
wwilsman
suggested changes
Oct 30, 2019
There was a problem hiding this comment.
Only real change request is the promise creation within async. It doesn't seem like that should be necessary since async () => {} implies () => return new Promise. Also though, loops are normally blocking unless the handlers we're testing are attached async (I don't think they are), so you might not even need the async at all for that loop.
The other two comments are nits
Robdel12
force-pushed
the
rd/winston-new-console-logger
branch
from
85160e8 to
d7b8559
Compare
wwilsman
approved these changes
Oct 30, 2019
djones
pushed a commit
that referenced
this pull request
Oct 30, 2019
## [0.19.5](v0.19.4...v0.19.5) (2019-10-30) ### Bug Fixes * Create a new console logger for each snapshot ([#407](#407)) ([f63b832](f63b832))
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is this?
After extensive debugging, I was able to trace
MaxListenersExceededWarning(#402) back to Winston and our console logger: winstonjs/winston#1334On lower resource machines, we were sending more logs than the stream could handle. To fix this, we now create a console logger for each snapshot, so they each have their own stream to operate on.