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

async_hooks,test: add test for async hooks parity for async/await #20626

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion test/async-hooks/hook-checks.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ exports.checkInvocations = function checkInvocations(activity, hooks, stage) {
);

// Check that actual invocations for all hooks match the expected invocations
[ 'init', 'before', 'after', 'destroy' ].forEach(checkHook);
[ 'init', 'before', 'after', 'destroy', 'promiseResolve' ].forEach(checkHook);

function checkHook(k) {
const val = hooks[k];
Expand Down
15 changes: 14 additions & 1 deletion test/async-hooks/init-hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class ActivityCollector {
onbefore,
onafter,
ondestroy,
onpromiseResolve,
logid = null,
logtype = null
} = {}) {
Expand All @@ -39,13 +40,16 @@ class ActivityCollector {
this.onbefore = typeof onbefore === 'function' ? onbefore : noop;
this.onafter = typeof onafter === 'function' ? onafter : noop;
this.ondestroy = typeof ondestroy === 'function' ? ondestroy : noop;
this.onpromiseResolve = typeof onpromiseResolve === 'function' ?
onpromiseResolve : noop;

// Create the hook with which we'll collect activity data
this._asyncHook = async_hooks.createHook({
init: this._init.bind(this),
before: this._before.bind(this),
after: this._after.bind(this),
destroy: this._destroy.bind(this)
destroy: this._destroy.bind(this),
promiseResolve: this._promiseResolve.bind(this)
});
}

Expand Down Expand Up @@ -206,6 +210,13 @@ class ActivityCollector {
this.ondestroy(uid);
}

_promiseResolve(uid) {
const h = this._getActivity(uid, 'promiseResolve');
this._stamp(h, 'promiseResolve');
this._maybeLog(uid, h && h.type, 'promiseResolve');
this.onpromiseResolve(uid);
}

_maybeLog(uid, type, name) {
if (this._logid &&
(type == null || this._logtype == null || this._logtype === type)) {
Expand All @@ -219,6 +230,7 @@ exports = module.exports = function initHooks({
onbefore,
onafter,
ondestroy,
onpromiseResolve,
allowNoInit,
logid,
logtype
Expand All @@ -228,6 +240,7 @@ exports = module.exports = function initHooks({
onbefore,
onafter,
ondestroy,
onpromiseResolve,
allowNoInit,
logid,
logtype
Expand Down
90 changes: 90 additions & 0 deletions test/async-hooks/test-async-await.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
'use strict';
const common = require('../common');

// This test ensures async hooks are being properly called
// when using async-await mechanics. This involves:
// 1. Checking that all initialized promises are being resolved
// 2. Checking that for each 'before' corresponding hook 'after' hook is called

const assert = require('assert');
const initHooks = require('./init-hooks');

const util = require('util');

const sleep = util.promisify(setTimeout);
// either 'inited' or 'resolved'
const promisesInitState = new Map();
// either 'before' or 'after' AND asyncId must be present in the other map
const promisesExecutionState = new Map();

const hooks = initHooks({
oninit,
onbefore,
onafter,
ondestroy: null, // Intentionally not tested, since it will be removed soon
onpromiseResolve
});
hooks.enable();

function oninit(asyncId, type, triggerAsyncId, resource) {
if (type === 'PROMISE') {
promisesInitState.set(asyncId, 'inited');
}
}

function onbefore(asyncId) {
if (!promisesInitState.has(asyncId)) {
return;
}
promisesExecutionState.set(asyncId, 'before');
}

function onafter(asyncId) {
if (!promisesInitState.has(asyncId)) {
return;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should remove things from the map here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check if this still applies to the new version - now the maps are more meant to reflect the state of the promises created, so IMO no need to clean them in the hooks.


assert.strictEqual(promisesExecutionState.get(asyncId), 'before',
'after hook called for promise without prior call' +
'to before hook');
assert.strictEqual(promisesInitState.get(asyncId), 'resolved',
'after hook called for promise without prior call' +
'to resolve hook');
promisesExecutionState.set(asyncId, 'after');
}

function onpromiseResolve(asyncId) {
assert(promisesInitState.has(asyncId),
'resolve hook called for promise without prior call to init hook');

promisesInitState.set(asyncId, 'resolved');
}

const timeout = common.platformTimeout(10);

function checkPromisesInitState() {
for (const initState of promisesInitState.values()) {
assert.strictEqual(initState, 'resolved',
'promise initialized without being resolved');
}
}

function checkPromisesExecutionState() {
for (const executionState of promisesExecutionState.values()) {
assert.strictEqual(executionState, 'after',
'mismatch between before and after hook calls');
}
}

process.on('beforeExit', common.mustCall(() => {
hooks.disable();

checkPromisesInitState();
checkPromisesExecutionState();
}));

async function asyncFunc(callback) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused callback param

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, removed.

await sleep(timeout);
}

asyncFunc();