From efe3b5b1a3f4fd50271f7171b18e20b479c8cfe9 Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Wed, 7 Feb 2018 20:05:45 +0100 Subject: [PATCH 1/2] domain: runtime deprecate MakeCallback Users of MakeCallback that adds the domain property to carry context, should start using the async_context variant of MakeCallback or the AsyncResource class. --- doc/api/deprecations.md | 9 +++++ lib/domain.js | 17 +++++++++ .../make-callback-domain-warning/binding.cc | 31 ++++++++++++++++ .../make-callback-domain-warning/binding.gyp | 9 +++++ .../make-callback-domain-warning/test.js | 35 +++++++++++++++++++ 5 files changed, 101 insertions(+) create mode 100644 test/addons/make-callback-domain-warning/binding.cc create mode 100644 test/addons/make-callback-domain-warning/binding.gyp create mode 100644 test/addons/make-callback-domain-warning/test.js diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index a9de845146e956..9426a612f131ef 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. + +### 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..98c230ba2b2c9c 100644 --- a/lib/domain.js +++ b/lib/domain.js @@ -29,6 +29,7 @@ const util = require('util'); const EventEmitter = require('events'); const errors = require('internal/errors'); +const internalUtil = require('internal/util'); const { createHook } = require('async_hooks'); // overwrite process.domain with a getter/setter that will allow for more @@ -94,13 +95,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', 'DEP00XX'); + 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..2079313901c756 --- /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, 'DEP00XX'); + })); + })); + })); +})); From 13facfa0ec502114bbbe8fb3bc3de7e11e849500 Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Thu, 8 Feb 2018 10:09:44 +0100 Subject: [PATCH 2/2] [squash] lint --- lib/domain.js | 1 - test/addons/make-callback-domain-warning/test.js | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/domain.js b/lib/domain.js index 98c230ba2b2c9c..260daed375d2e7 100644 --- a/lib/domain.js +++ b/lib/domain.js @@ -29,7 +29,6 @@ const util = require('util'); const EventEmitter = require('events'); const errors = require('internal/errors'); -const internalUtil = require('internal/util'); const { createHook } = require('async_hooks'); // overwrite process.domain with a getter/setter that will allow for more diff --git a/test/addons/make-callback-domain-warning/test.js b/test/addons/make-callback-domain-warning/test.js index 2079313901c756..fddbb4f2dc471d 100644 --- a/test/addons/make-callback-domain-warning/test.js +++ b/test/addons/make-callback-domain-warning/test.js @@ -5,9 +5,9 @@ const assert = require('assert'); const domain = require('domain'); const binding = require(`./build/${common.buildType}/binding`); -function makeCallback (object, cb) { - binding.makeCallback(object, () => setImmediate(cb)) -}; +function makeCallback(object, cb) { + binding.makeCallback(object, () => setImmediate(cb)); +} let latestWarning = null; process.on('warning', function(warning) {