diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index a5386f7bf015af..1f5809624afe95 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -871,6 +871,15 @@ Type: Runtime `timers.unenroll()` is deprecated. Please use the publicly documented [`clearTimeout()`][] or [`clearInterval()`][] instead. + +### DEP0097: MakeCallback with domain property + +Type: Runtime + +Users of `MakeCallback` that add the `domain` property to carry context, +should start using the `async_context` variant of `MakeCallback` or +`CallbackScope`, or the the high-level `AsyncResource` class. + [`--pending-deprecation`]: cli.html#cli_pending_deprecation [`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size [`Buffer.from(array)`]: buffer.html#buffer_class_method_buffer_from_array diff --git a/lib/domain.js b/lib/domain.js index 08fbd207f171d3..be109c9ce056bd 100644 --- a/lib/domain.js +++ b/lib/domain.js @@ -94,13 +94,29 @@ process.setUncaughtExceptionCaptureCallback = function(fn) { throw err; }; + +let sendMakeCallbackDeprecation = false; +function emitMakeCallbackDeprecation() { + if (!sendMakeCallbackDeprecation) { + process.emitWarning( + 'Using a domain property in MakeCallback is deprecated. Use the ' + + 'async_context variant of MakeCallback or the AsyncResource class ' + + 'instead.', 'DeprecationWarning', 'DEP0097'); + sendMakeCallbackDeprecation = true; + } +} + function topLevelDomainCallback(cb, ...args) { const domain = this.domain; + if (exports.active && domain) + emitMakeCallbackDeprecation(); + if (domain) domain.enter(); const ret = Reflect.apply(cb, this, args); if (domain) domain.exit(); + return ret; } diff --git a/test/addons/make-callback-domain-warning/binding.cc b/test/addons/make-callback-domain-warning/binding.cc new file mode 100644 index 00000000000000..c42166c7455277 --- /dev/null +++ b/test/addons/make-callback-domain-warning/binding.cc @@ -0,0 +1,31 @@ +#include "node.h" +#include "v8.h" + +#include + +using v8::Function; +using v8::FunctionCallbackInfo; +using v8::Isolate; +using v8::Local; +using v8::Object; +using v8::Value; + +namespace { + +void MakeCallback(const FunctionCallbackInfo& args) { + assert(args[0]->IsObject()); + assert(args[1]->IsFunction()); + Isolate* isolate = args.GetIsolate(); + Local recv = args[0].As(); + Local method = args[1].As(); + + node::MakeCallback(isolate, recv, method, 0, nullptr); +} + +void Initialize(Local exports) { + NODE_SET_METHOD(exports, "makeCallback", MakeCallback); +} + +} // namespace + +NODE_MODULE(NODE_GYP_MODULE_NAME, Initialize) diff --git a/test/addons/make-callback-domain-warning/binding.gyp b/test/addons/make-callback-domain-warning/binding.gyp new file mode 100644 index 00000000000000..7ede63d94a0d77 --- /dev/null +++ b/test/addons/make-callback-domain-warning/binding.gyp @@ -0,0 +1,9 @@ +{ + 'targets': [ + { + 'target_name': 'binding', + 'defines': [ 'V8_DEPRECATION_WARNINGS=1' ], + 'sources': [ 'binding.cc' ] + } + ] +} diff --git a/test/addons/make-callback-domain-warning/test.js b/test/addons/make-callback-domain-warning/test.js new file mode 100644 index 00000000000000..2ea3c3f3d14b2b --- /dev/null +++ b/test/addons/make-callback-domain-warning/test.js @@ -0,0 +1,35 @@ +'use strict'; + +const common = require('../../common'); +const assert = require('assert'); +const domain = require('domain'); +const binding = require(`./build/${common.buildType}/binding`); + +function makeCallback(object, cb) { + binding.makeCallback(object, () => setImmediate(cb)); +} + +let latestWarning = null; +process.on('warning', function(warning) { + latestWarning = warning; +}); + +const d = domain.create(); + +// When domain is disabled, no warning will be emitted +makeCallback({ domain: d }, common.mustCall(function() { + assert.strictEqual(latestWarning, null); + + d.run(common.mustCall(function() { + // No warning will be emitted when no domain property is applied + makeCallback({}, common.mustCall(function() { + assert.strictEqual(latestWarning, null); + + // Warning is emitted when domain property is used and domain is enabled + makeCallback({ domain: d }, common.mustCall(function() { + assert.strictEqual(latestWarning.name, 'DeprecationWarning'); + assert.strictEqual(latestWarning.code, 'DEP0097'); + })); + })); + })); +}));