FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

domain: set `.domain` non-enumerable on resources · nodejs/node@56adebf · GitHub

/ node Public

Commit 56adebf

Browse files
ljharb
authored and
BridgeAR
committed
domain: set .domain non-enumerable on resources
In particular, this comes into play in the node repl, which apparently enables domains by default. Whenever any Promise gets inspected, a `.domain` property is displayed, which is *very confusing*, especially since it has some kind of WeakReference attached to it, which is not yet a language feature. This change will prevent it from showing up in casual inspection, but will leave it available for use. PR-URL: #26210 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
1 parent 6f68570 commit 56adebf

5 files changed

Lines changed: 59 additions & 8 deletions

File tree

‎lib/domain.js‎

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,23 @@ const asyncHook = createHook({
5858
if (process.domain !== null && process.domain !== undefined) {
5959
// If this operation is created while in a domain, let's mark it
6060
pairing.set(asyncId, process.domain[kWeak]);
61-
resource.domain = process.domain;
61+
Object.defineProperty(resource, 'domain', {
62+
configurable: true,
63+
enumerable: false,
64+
value: process.domain,
65+
writable: true
66+
});
6267
if (resource.promise !== undefined &&
6368
resource.promise instanceof Promise) {
6469
// resource.promise instanceof Promise make sure that the
6570
// promise comes from the same context
6671
// see https://github.com/nodejs/node/issues/15673
67-
resource.promise.domain = process.domain;
72+
Object.defineProperty(resource.promise, 'domain', {
73+
configurable: true,
74+
enumerable: false,
75+
value: process.domain,
76+
writable: true
77+
});
6878
}
6979
}
7080
},
@@ -203,7 +213,12 @@ Domain.prototype._errorHandler = function(er) {
203213
var caught = false;
204214

205215
if (!util.isPrimitive(er)) {
206-
er.domain = this;
216+
Object.defineProperty(er, 'domain', {
217+
configurable: true,
218+
enumerable: false,
219+
value: this,
220+
writable: true
221+
});
207222
er.domainThrown = true;
208223
}
209224

@@ -320,7 +335,12 @@ Domain.prototype.add = function(ee) {
320335
}
321336
}
322337

323-
ee.domain = this;
338+
Object.defineProperty(ee, 'domain', {
339+
configurable: true,
340+
enumerable: false,
341+
value: this,
342+
writable: true
343+
});
324344
this.members.push(ee);
325345
};
326346

@@ -359,7 +379,12 @@ function intercepted(_this, self, cb, fnargs) {
359379
var er = fnargs[0];
360380
er.domainBound = cb;
361381
er.domainThrown = false;
362-
er.domain = self;
382+
Object.defineProperty(er, 'domain', {
383+
configurable: true,
384+
enumerable: false,
385+
value: self,
386+
writable: true
387+
});
363388
self.emit('error', er);
364389
return;
365390
}
@@ -413,7 +438,12 @@ Domain.prototype.bind = function(cb) {
413438
return bound(this, self, cb, arguments);
414439
}
415440

416-
runBound.domain = this;
441+
Object.defineProperty(runBound, 'domain', {
442+
configurable: true,
443+
enumerable: false,
444+
value: this,
445+
writable: true
446+
});
417447

418448
return runBound;
419449
};
@@ -423,7 +453,12 @@ EventEmitter.usingDomains = true;
423453

424454
const eventInit = EventEmitter.init;
425455
EventEmitter.init = function() {
426-
this.domain = null;
456+
Object.defineProperty(this, 'domain', {
457+
configurable: true,
458+
enumerable: false,
459+
value: null,
460+
writable: true
461+
});
427462
if (exports.active && !(this instanceof exports.Domain)) {
428463
this.domain = exports.active;
429464
}
@@ -452,7 +487,12 @@ EventEmitter.prototype.emit = function(...args) {
452487

453488
if (typeof er === 'object') {
454489
er.domainEmitter = this;
455-
er.domain = domain;
490+
Object.defineProperty(er, 'domain', {
491+
configurable: true,
492+
enumerable: false,
493+
value: domain,
494+
writable: true
495+
});
456496
er.domainThrown = false;
457497
}
458498

‎test/parallel/test-domain-add-remove.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,15 @@ require('../common');
44
const assert = require('assert');
55
const domain = require('domain');
66
const EventEmitter = require('events');
7+
const isEnumerable = Function.call.bind(Object.prototype.propertyIsEnumerable);
78

89
const d = new domain.Domain();
910
const e = new EventEmitter();
1011
const e2 = new EventEmitter();
1112

1213
d.add(e);
1314
assert.strictEqual(e.domain, d);
15+
assert.strictEqual(isEnumerable(e, 'domain'), false);
1416

1517
// Adding the same event to a domain should not change the member count
1618
let previousMemberCount = d.members.length;
@@ -19,8 +21,10 @@ assert.strictEqual(previousMemberCount, d.members.length);
1921

2022
d.add(e2);
2123
assert.strictEqual(e2.domain, d);
24+
assert.strictEqual(isEnumerable(e2, 'domain'), false);
2225

2326
previousMemberCount = d.members.length;
2427
d.remove(e2);
2528
assert.notStrictEqual(e2.domain, d);
29+
assert.strictEqual(isEnumerable(e2, 'domain'), false);
2630
assert.strictEqual(previousMemberCount - 1, d.members.length);

‎test/parallel/test-domain-async-id-map-leak.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ const assert = require('assert');
66
const async_hooks = require('async_hooks');
77
const domain = require('domain');
88
const EventEmitter = require('events');
9+
const isEnumerable = Function.call.bind(Object.prototype.propertyIsEnumerable);
910

1011
// This test makes sure that the (async id → domain) map which is part of the
1112
// domain module does not get in the way of garbage collection.
@@ -21,7 +22,9 @@ d.run(() => {
2122

2223
emitter.linkToResource = resource;
2324
assert.strictEqual(emitter.domain, d);
25+
assert.strictEqual(isEnumerable(emitter, 'domain'), false);
2426
assert.strictEqual(resource.domain, d);
27+
assert.strictEqual(isEnumerable(resource, 'domain'), false);
2528

2629
// This would otherwise be a circular chain now:
2730
// emitter → resource → async id ⇒ domain → emitter.

‎test/parallel/test-domain-implicit-binding.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,15 @@ const common = require('../common');
44
const assert = require('assert');
55
const domain = require('domain');
66
const fs = require('fs');
7+
const isEnumerable = Function.call.bind(Object.prototype.propertyIsEnumerable);
78

89
{
910
const d = new domain.Domain();
1011

1112
d.on('error', common.mustCall((err) => {
1213
assert.strictEqual(err.message, 'foobar');
1314
assert.strictEqual(err.domain, d);
15+
assert.strictEqual(isEnumerable(err, 'domain'), false);
1416
assert.strictEqual(err.domainEmitter, undefined);
1517
assert.strictEqual(err.domainBound, undefined);
1618
assert.strictEqual(err.domainThrown, true);

‎test/parallel/test-domain-timer.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@
33
const common = require('../common');
44
const assert = require('assert');
55
const domain = require('domain');
6+
const isEnumerable = Function.call.bind(Object.prototype.propertyIsEnumerable);
67

78
const d = new domain.Domain();
89

910
d.on('error', common.mustCall((err) => {
1011
assert.strictEqual(err.message, 'foobar');
1112
assert.strictEqual(err.domain, d);
13+
assert.strictEqual(isEnumerable(err, 'domain'), false);
1214
assert.strictEqual(err.domainEmitter, undefined);
1315
assert.strictEqual(err.domainBound, undefined);
1416
assert.strictEqual(err.domainThrown, true);

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL