-
Notifications
You must be signed in to change notification settings - Fork 1
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
build(deps): bump winston-transport from 4.7.1 to 4.8.0 #121
build(deps): bump winston-transport from 4.7.1 to 4.8.0 #121
Conversation
CI Failure Feedback 🧐(Checks updated until commit 46c7734)
✨ CI feedback usage guide:The CI feedback tool (
In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:
where Configuration options
See more information about the |
Bumps [winston-transport](https://github.com/winstonjs/winston-transport) from 4.7.1 to 4.8.0. - [Release notes](https://github.com/winstonjs/winston-transport/releases) - [Changelog](https://github.com/winstonjs/winston-transport/blob/master/CHANGELOG.md) - [Commits](winstonjs/winston-transport@v4.7.1...v4.8.0) --- updated-dependencies: - dependency-name: winston-transport dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
f33e026
to
46c7734
Compare
@ramhr can you please take a look |
const logMessage = this._buildLog(info); | ||
|
||
this.sender.send(logMessage, (err) => { | ||
this.sender.send(info[MESSAGE], (err) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The info[MESSAGE]
is the formatted message as built in the format
phase of the winston-transport
pipeline. The info
itself is still the same log object.
@@ -185,7 +190,7 @@ class LogstashUDP extends Transport { | |||
} | |||
|
|||
_buildLog(info) { | |||
const meta = Object.assign({}, info, this.meta_defaults); | |||
const meta = Object.assign(info, this.meta_defaults); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
winston-transport
already does Object.assign({}, info)
before passing it to the format, so it's already a fresh object and no need to dup it again.
if (!this.format) { | ||
this.format = defaultFormat; | ||
} else { | ||
this.format = winston.format.combine(this.format, defaultFormat); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just in case someone passes a custom format in the options, merge it with the udp transport format.
|
||
expect(() => { | ||
logger.info("bad", { timer: setTimeout(() => {}, 10) }); | ||
logger.info("bad", { circular }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This gives the same affect, but is clearer to understand.
With the updated `winston-transport`, the transport write pipeline was changed, the place where the `log` method is called is different. Before, errors inside the log were always handled synchronously right after the write, but now in practice it's handled in `process.nextTick`. This still runs in the same tick of the event loop, but after the current operation is finished. As a side effect, this means that errors thrown there are not caught in the current operation's context and try/catch blocks wrapping it won't work. This means that in case of the format error which is tested in the broken unit test, no error is caught in the test so it fails. In fact, the logger does throw the error, but it's thrown only after the test finishes as an unhandled exception, which is very bas and I guess that's why this test exists. In order to fix the issue, I moved the message formatting into the `format` part of the winston-transport pipeline, instead of inside of the `log` function itself. When there is a formatting error inside this pipeline, winston catches it on it's own, handles it internally and rethrows it, so the log action would throw it and it can be caught.
8ae3e50
to
039107e
Compare
A newer version of winston-transport exists, but since this PR has been edited by someone other than Dependabot I haven't updated it. You'll get a PR for the updated version as normal once this PR is merged. |
@dependabot recreate |
Superseded by #131. |
Bumps winston-transport from 4.7.1 to 4.8.0.
Release notes
Sourced from winston-transport's releases.
Commits
75c5d2f
4.8.0000b11f
Bump readable-stream from 3.6.0 to 4.5.2 (#201)7e7dbc6
Bump nyc from 17.0.0 to 17.1.0 (#243)d85058c
Bump mocha from 10.6.0 to 10.7.3 (#233)f721b63
Bump@types/node
from 20.14.10 to 22.6.0 (#241)ff4f65e
Bump eslint from 9.6.0 to 9.11.0 (#242)fb8383e
fix tests (#244)You can trigger a rebase of this PR by commenting
@dependabot rebase
.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebase
will rebase this PR@dependabot recreate
will recreate this PR, overwriting any edits that have been made to it@dependabot merge
will merge this PR after your CI passes on it@dependabot squash and merge
will squash and merge this PR after your CI passes on it@dependabot cancel merge
will cancel a previously requested merge and block automerging@dependabot reopen
will reopen this PR if it is closed@dependabot close
will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot show <dependency name> ignore conditions
will show all of the ignore conditions of the specified dependency@dependabot ignore this major version
will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor version
will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependency
will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)