Commit Graph

6 Commits

Author SHA1 Message Date
Daniel Estiven Rico Posada
e99ec39610 test: test-async-wrap-constructor prefer forEach
PR-URL: https://github.com/nodejs/node/pull/32631
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
2020-04-23 19:32:35 -05:00
Ruben Bridgewater
e038d6a1cd
test: refactor common.expectsError
This completely refactors the `expectsError` behavior: so far it's
almost identical to `assert.throws(fn, object)` in case it was used
with a function as first argument. It had a magical property check
that allowed to verify a functions `type` in case `type` was passed
used in the validation object. This pattern is now completely removed
and `assert.throws()` should be used instead.

The main intent for `common.expectsError()` is to verify error cases
for callback based APIs. This is now more flexible by accepting all
validation possibilites that `assert.throws()` accepts as well. No
magical properties exist anymore. This reduces surprising behavior
for developers who are not used to the Node.js core code base.

This has the side effect that `common` is used significantly less
frequent.

PR-URL: https://github.com/nodejs/node/pull/31092
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
2019-12-31 15:54:20 +01:00
Mithun Sasidharan
146a9b1a8a
test: replace assert.throws w/ common.expectsError
PR-URL: https://github.com/nodejs/node/pull/17483
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2017-12-08 16:23:32 -05:00
Braden Whitten
ba999e1b9e
test: more AsyncWrap constructor validation tests
Fixes a line of uncovered code in the AsyncWrap class's constructor.
Specifically this covers validtion of the 'promiseResolve' argument.

PR-URL: https://github.com/nodejs/node/pull/16025
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
2017-10-19 02:26:02 -02:00
Trevor Norris
062beb08df
async_hooks: don't abort unnecessarily
* id values of -1 are allowed. They indicate that the id was never
  correctly assigned to the async resource. These will appear in any
  call graph, and will only be apparent to those using the async_hooks
  module, then reported in an issue.
* ids < -1 are still not allowed and will cause the application to
  exit the process; because there is no scenario where this should ever
  happen.
* Add asyncId range checks to emitAfterScript().
* Fix emitBeforeScript() range checks which should have been || not &&.
* Replace errors with entries in internal/errors.
* Fix async_hooks tests that check for exceptions to match new
  internal/errors entries.

NOTE: emit{Before,After,Destroy}() must continue to exit the process
because in the case of an exception during hook execution the state of
the application is unknowable. For example, an exception could cause a
memory leak:

    const id_map = new Map();

    before(id) {
      id_map.set(id, /* data object or similar */);
    },
    after(id) {
      throw new Error('id never dies!');
      id_map.delete(id);
    }

Allowing a recoverable exception may also cause an abort because of a
stack check in Environment::AsyncHooks::pop_ids() that verifies the
async id and pop'd ids match. This case would be more difficult to debug
than if fatalError() (lib/async_hooks.js) was called immediately.

    try {
      async_hooks.emitBefore(null, NaN);
    } catch (e) { }
    // do something
    async_hooks.emitAfter(5);

It also allows an edge case where emitBefore() could be called twice and
not have the pop_ids() CHECK fail:

    try {
      async_hooks.emitBefore(5, NaN);
    } catch (e) { }
    async_hooks.emitBefore(5);
    // do something
    async_hooks.emitAfter(5);

There is the option of allowing mismatches in the stack and ignoring the
check if no async hooks are enabled, but I don't believe going this far
is necessary.

PR-URL: https://github.com/nodejs/node/pull/14722
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
2017-08-14 23:21:01 +02:00
Shadowbeetle
6fb27af70a
async_hooks: add constructor check to async-hooks
This fixes the async_hooks.AsyncHook constructor such that it throws an
error when provided with falsy values other than undefined.

PR-URL: https://github.com/nodejs/node/pull/13096
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
2017-05-21 22:04:15 +02:00