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

events: show inspected error in uncaught 'error' message #25621

Closed
wants to merge 3 commits into from

Conversation

addaleax
Copy link
Member

If there is no handler for .emit('error', value) and value
is not an Error object, we currently just call .toString()
on it.

Almost always, using util.inspect() provides better information
for diagnostic purposes, so prefer to use that instead.

Refs: nodejs/help#1729

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

If there is no handler for `.emit('error', value)` and `value`
is not an `Error` object, we currently just call `.toString()`
on it.

Almost always, using `util.inspect()` provides better information
for diagnostic purposes, so prefer to use that instead.

Refs: nodejs/help#1729
@nodejs-github-bot nodejs-github-bot added the events Issues and PRs related to the events subsystem / EventEmitter. label Jan 21, 2019
lib/events.js Outdated Show resolved Hide resolved
lib/events.js Outdated Show resolved Hide resolved
@addaleax
Copy link
Member Author

@BridgeAR Done, thanks for the review!

CI: https://ci.nodejs.org/job/node-test-pull-request/20266/

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 22, 2019
addaleax added a commit that referenced this pull request Jan 23, 2019
If there is no handler for `.emit('error', value)` and `value`
is not an `Error` object, we currently just call `.toString()`
on it.

Almost always, using `util.inspect()` provides better information
for diagnostic purposes, so prefer to use that instead.

Refs: nodejs/help#1729

PR-URL: #25621
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
@addaleax
Copy link
Member Author

Landed in eeea0dd

@addaleax addaleax closed this Jan 23, 2019
@addaleax addaleax deleted the events-error-stringify branch January 23, 2019 15:51
addaleax added a commit that referenced this pull request Jan 23, 2019
If there is no handler for `.emit('error', value)` and `value`
is not an `Error` object, we currently just call `.toString()`
on it.

Almost always, using `util.inspect()` provides better information
for diagnostic purposes, so prefer to use that instead.

Refs: nodejs/help#1729

PR-URL: #25621
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2019
MylesBorins added a commit that referenced this pull request Jan 24, 2019
Notable Changes:

* events:
  * For unhandled `error` events with an argument that is not an
    `Error` object, the resulting exeption will have more information
    about the argument.
    #25621
* child_process:
  * When the `maxBuffer` option is passed, `stdout` and `stderr` will
    be truncated rather than unavailable in case of an error.
    #24951
* policy:
  * Experimental support for module integrity checks through a manifest
    file is implemented now.
    #23834
* n-api:
  * The `napi_threadsafe_function` feature is now stable.
    #25556
* report:
  * An experimental diagnostic API for capturing process state is
    available as `process.report` and through command line flags.
    #22712
* tls:
  * `tls.connect()` takes a `timeout` option analogous to the
    `net.connect()` one.
    #25517
* worker:
  * `process.umask()` is available as a read-only function inside Worker
    threads now.
    #25526
  * An `execArgv` option that supports a subset of Node.js command line
    options is supported now.
    #25467

PR-URL: #25687
MylesBorins added a commit that referenced this pull request Jan 25, 2019
Notable Changes:

* events:
  * For unhandled `error` events with an argument that is not an
    `Error` object, the resulting exeption will have more information
    about the argument.
    #25621
* child_process:
  * When the `maxBuffer` option is passed, `stdout` and `stderr` will
    be truncated rather than unavailable in case of an error.
    #24951
* policy:
  * Experimental support for module integrity checks through a manifest
    file is implemented now.
    #23834
* n-api:
  * The `napi_threadsafe_function` feature is now stable.
    #25556
* report:
  * An experimental diagnostic API for capturing process state is
    available as `process.report` and through command line flags.
    #22712
* tls:
  * `tls.connect()` takes a `timeout` option analogous to the
    `net.connect()` one.
    #25517
* worker:
  * `process.umask()` is available as a read-only function inside Worker
    threads now.
    #25526
  * An `execArgv` option that supports a subset of Node.js command line
    options is supported now.
    #25467

PR-URL: #25687
BethGriggs pushed a commit that referenced this pull request Apr 29, 2019
If there is no handler for `.emit('error', value)` and `value`
is not an `Error` object, we currently just call `.toString()`
on it.

Almost always, using `util.inspect()` provides better information
for diagnostic purposes, so prefer to use that instead.

Refs: nodejs/help#1729

PR-URL: #25621
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
@BethGriggs BethGriggs mentioned this pull request May 1, 2019
BethGriggs pushed a commit that referenced this pull request May 10, 2019
If there is no handler for `.emit('error', value)` and `value`
is not an `Error` object, we currently just call `.toString()`
on it.

Almost always, using `util.inspect()` provides better information
for diagnostic purposes, so prefer to use that instead.

Refs: nodejs/help#1729

PR-URL: #25621
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
If there is no handler for `.emit('error', value)` and `value`
is not an `Error` object, we currently just call `.toString()`
on it.

Almost always, using `util.inspect()` provides better information
for diagnostic purposes, so prefer to use that instead.

Refs: nodejs/help#1729

PR-URL: #25621
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. events Issues and PRs related to the events subsystem / EventEmitter.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants