[v12.x backport] events: allow monitoring error events by Flarna · Pull Request #32004 · nodejs/node

@nodejs-github-bot added events

Issues and PRs related to the events subsystem / EventEmitter.

v12.x labels

Feb 28, 2020

@targos targos added the semver-minor

PRs that contain new features and should be released in the next minor version.

label

Mar 7, 2020

BridgeAR

Installing an error listener has a side effect that emitted errors are
considered as handled. This is quite bad for monitoring/logging tools
which tend to be interested in errors but don't want to cause side
effects like swallow an exception.

There are some workarounds in the wild like monkey patching emit or
remit the error if monitoring tool detects that it is the only listener
but this is error prone and risky.

This PR allows to install a listener to monitor errors with the side
effect to consume the error. To avoid conflicts with other events it
exports a symbol on EventEmitter which owns this special meaning.

Refs: open-telemetry/opentelemetry-js#225

PR-URL: nodejs#30932
Refs: open-telemetry/opentelemetry-js#225
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Convert property errorMonitor to a normal property as non-writable
caused unwanted side effects.

Refs: nodejs#30932 (comment)

@Flarna

@Flarna

@Flarna Flarna added the author ready

PRs that have at least one approval, no pending requests for changes, and a CI started.

label

Apr 8, 2020

targos pushed a commit to targos/node that referenced this pull request

Apr 25, 2020
Installing an error listener has a side effect that emitted errors are
considered as handled. This is quite bad for monitoring/logging tools
which tend to be interested in errors but don't want to cause side
effects like swallow an exception.

There are some workarounds in the wild like monkey patching emit or
remit the error if monitoring tool detects that it is the only listener
but this is error prone and risky.

This PR allows to install a listener to monitor errors with the side
effect to consume the error. To avoid conflicts with other events it
exports a symbol on EventEmitter which owns this special meaning.

Refs: open-telemetry/opentelemetry-js#225

Backport-PR-URL: nodejs#32004
PR-URL: nodejs#30932
Refs: open-telemetry/opentelemetry-js#225
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

targos pushed a commit that referenced this pull request

Apr 28, 2020
Installing an error listener has a side effect that emitted errors are
considered as handled. This is quite bad for monitoring/logging tools
which tend to be interested in errors but don't want to cause side
effects like swallow an exception.

There are some workarounds in the wild like monkey patching emit or
remit the error if monitoring tool detects that it is the only listener
but this is error prone and risky.

This PR allows to install a listener to monitor errors with the side
effect to consume the error. To avoid conflicts with other events it
exports a symbol on EventEmitter which owns this special meaning.

Refs: open-telemetry/opentelemetry-js#225

Backport-PR-URL: #32004
PR-URL: #30932
Refs: open-telemetry/opentelemetry-js#225
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

@Flarna Flarna deleted the v12-backport-eventmonitor branch

June 3, 2020 08:25