deprecate domains

what's the status on domains? can we deprecate it before v1?

afaik no breaking changes will happen until v1

just labeling something as deprecated is not a breaking change

gotcha, 👍 from me, domains have only meant pain to me personally

I'm on board, I bet a bunch of TC members would be too, marking as tc-agenda

+1

I never understood why domains were added to core.

It seemed wrong at the time, and still seems equally as wrong.

For backwards compatibility, will it be possible to put this functionality into an npm module as a custom add-on? Does this already exist?

+1 domains are confusing and I hear nothing but incompatibilities and edge cases

@Marak

+1 sad 🎺

@Marak

I never understood why domains were added to core.

There's a place for unified sync & async error handling for Node, and due to the way that state propagates through core, there at least need to be hooks exposed by core to facilitate this. uncaughtException is too gross-grained, not everybody uses (or likes) promises, try/catch + stream error listeners + callback convention is complicated to teach and learn and harder to master, etc. Probably the best effort at this so far is @piscisaureus and co's zones, but that solution has limitations that are going to be tough to resolve outside core.

I'm hopeful that @trevnorris's AsyncWrap can provide the foundation for a better / lower-impact way of building something useful along these lines in the future, and I consider domains a very useful failure that helped point to what AsyncWrap / AsyncListener should do.

It would have to go through the TC but I don't think there is much opposition to deprecating domains. Who volunteers to follow up with a pull request?

@bnoordhuis

@bnoordhuis A deprecation warning if you call domain.create()? Or when you require it since it has side effects (acording to code comments).

@tellnes

@tellnes I'll leave that to your own judgment. Doing it at require() time would get the message out most effectively but it might end up being extremely annoying.

If you do do it at require() time, you should also update lib/repl.js and make it load the domain module lazily.

lib/repl.js needs to be changed anyway because it is calling domain.create().

Is there a suitable replacement that could be used? Repl is a valid use-case for example.

@rlidwka

@rlidwka Yes, the new tracing module in core. Please see my implementation in #75.

I haven't read the whole thread, so sorry if I'm repeating things.

I want to say that I've recently been using domains to capture all calls to console.log, console.warn, etc. It is not a simple matter of monkey-patching these methods, since I also need to separate each call to its source (which is a unique context defined by my application).

So, I'm not using domains for handling errors at all. I don't even register an error event listener on my domains. I'm using domains as a way of execution context management.

This feature is necessary for my application to work, and so I hope that any replacement API to Domain will be able to cover my use-case...

BTW, I would be glad to know if there's a better way to do what I'm trying to do, other than abusing domains.

EDIT
I just wanted to add that it's not possible for me to improve my application to keep track of context manually, as my application keeps track of third-party user-code.

N.B.
I've also encountered the issues with native Promises. In my case, I fixed it by monkey-patching Promise (even though it might have a big performance hit):

function deferResult(fn, ...args) {
    let hasError = false;
    let error;
    let result;

    try {
        result = fn(...args);
    } catch (err) {
        hasError = true;
        error = err;
    }

    return () => {
        if (hasError) { throw error; }
        return result;
    };
}

const originalThen = Promise.prototype.then;
Promise.prototype.then = function (onFulfilled, onRejected) {
    const activeDomain = domain.active;

    let onF = onFulfilled;
    if (activeDomain && typeof onFulfilled === 'function') {
        onF = function (value) {
            activeDomain.enter();
            const deferredResult = deferResult(onFulfilled, value);
            activeDomain.exit();
            return deferredResult();
        };
    }

    let onR = onRejected;
    if (activeDomain && typeof onRejected === 'function') {
        onR = function (reason) {
            activeDomain.enter();
            const deferredResult = deferResult(onRejected, reason);
            activeDomain.exit();
            return deferredResult();
        };
    }

    Reflect.apply(originalThen, this, [onF, onR]);
};

@SosZ

@SosZ please search and read the whole thread before, I patched promises to use domains myself, in a much simpler way

Your code is a little bit of a horror show. What about this instead?

const then = Promise.prototype.then;
Promise.prototype.then = function (fn1, fn2) {

  if (process.domain) {
    fn1 = fn1 && process.domain.bind(fn1);
    fn2 = fn2 && process.domain.bind(fn2);
  }

  return then.call(this, fn1, fn2);
};

Note that you do not need to patch the Promise constructor, because the executors run synchronously, but you can patch Promise.prototype.catch to work in the same way as the above patch on Promise.prototype.then. That being said, pretty certain Node version 8 has allowed domains to work with Promises, so the above patch may only be useful for Node.js versions < 8.

I just want to add my 2 cents to the long run discussion. I have a job. I want to keep my job. I really prefer to not have my server(s) crash, even if we use forever or supervisor to restart upon crashes.

Using the following allows us to avoid many crashes:

let domain = require('domain');

app.use(function(req, res, next){

  let d = domain.create(); // create a new domain for this request

  res.once('finish', function(){
    d.exit();
    d.removeAllListeners();
  });

  d.once('error', function(e){
    // log the issue right here
    if(!res.headersSent){
        res.json({
          error: e.stack || e
       });
    }
  });

  d.run(next);   // we invoke the next middleware
});

we use domains in production. Hopefully no memory leaks though 😂

@ORESoftware

@ORESoftware I've moderated your previous comment. I found it to be inappropriate for this issue tracker. We intend on keeping this issue tracker professional and would appreciate if everyone who posts here does the same.

Domains are still a hack, letting the process crash is often the better option especially with forever/supervisor.

@evanlucas

@evanlucas censorship always appreciated, but I guess you're right, best to keep politics off of Github lol...@nickmmcurdy - you'd have to back up that statement - domains will fail in their intention if an error is raised for the wrong path of execution, and if it's easy to misuse them than that's also bad. But I don't think they are a hack perse, just perhaps imperfectly implemented.

@ORESoftware

@ORESoftware We have a Code of Conduct that asks you to be respectful of other people’s viewpoints and refrain from using language that “could reasonably be considered inappropriate in a professional setting”.

If you don’t feel comfortable with that, it might be easier to take a step back; Node’s project leadership is standing behind the choice to apply that CoC to our spaces.

Imma read the CoC right now to see if my microaggression actually violated it.

Yep, looks like I violated point 1

Using welcoming and inclusive language

whoops, my bad fam

deps: V8: cherry-pick d2283ba066ba

deps: V8: cherry-pick 813066946968

deps: V8: cherry-pick d2283ba066ba

deps: V8: cherry-pick 813066946968

Navigation Menu

Repository navigation

deprecate domains #66

84 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

deprecate domains #66

Description

Activity

juliangruber commented on Dec 4, 2014

jonathanong commented on Dec 4, 2014

juliangruber commented on Dec 4, 2014

rvagg commented on Dec 4, 2014

indutny commented on Dec 4, 2014

Marak commented on Dec 4, 2014

Fishrock123 commented on Dec 4, 2014

othiym23 commented on Dec 4, 2014

bnoordhuis commented on Dec 4, 2014

tellnes commented on Dec 4, 2014

bnoordhuis commented on Dec 4, 2014

tellnes commented on Dec 5, 2014

rlidwka commented on Dec 5, 2014

tellnes commented on Dec 5, 2014

84 remaining items

bayov commented on Jul 23, 2016

ORESoftware commented on Aug 4, 2017

ORESoftware commented on Aug 4, 2017

evanlucas commented on Aug 8, 2017

nickserv commented on Aug 8, 2017

ORESoftware commented on Aug 8, 2017

addaleax commented on Aug 8, 2017

ORESoftware commented on Aug 8, 2017

ORESoftware commented on Aug 8, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions