domain is not preserved correctly when there is no domain

[emptyemail]

If a connection is created outside of a domain, connected inside a domain, and later used outside the domain again, it will still invoke callbacks on the domain even though the domain is now out of scope.

Here is a test case that replicates

d = require('domain').createDomain()
mysql = require('mysql')
connection = mysql.createConnection({host: 'localhost', user:'dev', password: 'pass'})
d.run ->
  connection.connect()
checkDomain = ->
  should.not.exist(process.domain)
  connection.query "SELECT 1", (err, results) ->
    should.not.exist(process.domain)
    done()
setTimeout(checkDomain, 100)

modifying bindToCurrentDomain to force no domain did the trick.

function bindToCurrentDomain(callback) {
  if (!callback) return;

  var domain = process.domain;
  forceNoDomain = function() {
    while(process.domain)
      process.domain.exit()
    callback.apply(this, arguments)
  }
  return domain
    ? domain.bind(callback)
    : forceNoDomain;
}

I am open to creating a pull request for this, but want to get some feedback first.

[dougwilson]

I have never used domains, rather the current implementation was donated via a PR :) Domains in Node.js have been deprecated for a while (https://nodejs.org/api/domain.html), so I'm not sure if this is a bug in domains themselves or here. Please feel free to provide a PR and explain from the Node.js documentation why a loop of domain.exit() is necessary and is truly the correct solution for this.

[sidorares]

Hey @emptyemail - this was originally my PR - #709 . I'd agree with @dougwilson - as domains are marked as deprecated maybe it's worth removing completely

[emptyemail]

Hey @sidorares and @dougwilson. Even though domains are deprecated, I suspect it will be a long while before we can safely remove support, unless we are ok with a breaking change.

Also that fix is not 100% accurate, as you would need to restore the original domain stack after the callback is invoked. I think the concept of the bind function belongs in the core domain implementation. The reason for the domain.exit is that's the only way to execute a callback outside the domain. The way I see it, if you create an emitter as part of the domain it will preserve the domain, if you create one outside the domain it should stay outside the domain and not get assigned to an arbitrary one. I am trying to think of a scenario when you would not want that, but nothing comes to mind. I was hoping someone that has used domains would chime in.

[dougwilson]

AFAIK domains were deprecated in core because they just never worked right or as expected, just like in this case.

[dougwilson]

Hi @emptyemail, I'm going to close this because I don't use domains and there still hasn't been a PR made for this is over 6 months since this issue, so it seems no one is interested in solving this issue. From what I could tell from researching a bit, because you called .connect() inside a domain, the underlying socket we created with net.createConnection ends up implicitly bound to the domain, and thus everything coming from that socket's events are bound to it, which is why it ends up getting tied to all the future callbacks.

[dougwilson]

So looking into this myself, it looks like one possible solution can be to at least get the underlying socket to attach to the same domain as the connection object, which would cause your test case above pass. In addition, I think it's just in general much less surprising, as I feel like after reading the documentation for domains, having the fact that we lazily create the underlying socket creep out in the way of an unknown domain is just confusing.