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.

Sign up for GitHub

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

Jump to bottom

[v12.x backport] events: allow monitoring error events #32004

Closed
wants to merge 4 commits into from
Closed

[v12.x backport] events: allow monitoring error events #32004

wants to merge 4 commits into from
+64 −2

Commits on Apr 6, 2020

  1. events: allow monitoring error events 

    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>
    @Flarna
    Flarna committed Apr 6, 2020
    Configuration menu
    Copy the full SHA
    1c14d65 View commit details
    Browse the repository at this point in the history

  2. events: convert errorMonitor to a normal property 

    Convert property errorMonitor to a normal property as non-writable
caused unwanted side effects.

Refs: nodejs#30932 (comment)
    @Flarna
    Flarna committed Apr 6, 2020
    Configuration menu
    Copy the full SHA
    c483e6d View commit details
    Browse the repository at this point in the history

  3. fixup! events: allow monitoring error events

    @Flarna
    Flarna committed Apr 6, 2020
    Configuration menu
    Copy the full SHA
    a9356f4 View commit details
    Browse the repository at this point in the history

  4. change common.expectsError to assert.throws

    @Flarna
    Flarna committed Apr 6, 2020
    Configuration menu
    Copy the full SHA
    4912594 View commit details
    Browse the repository at this point in the history