Client modifies configuration object such that it cannot be used again

[jvonniedapn]

When passing a config object to new elasticsearch.Client(), the Client modifies the passed in config object in such a way that it cannot be used to create a second client.

Example:

var elasticsearch = require('elasticsearch');

var config = {
    log : "warning"
};

console.dir(config);
var es1 = new elasticsearch.Client(config);
console.dir(config);
var es2 = new elasticsearch.Client(config);

> node es-config-bug.js 
{ log: 'warning' }
{ log: 
   { _events: 
      { closing: [Object],
        error: [Function: bound],
        warning: [Function: bound] } },
  host: 'http://localhost:9200',
  hosts: 'http://localhost:9200',
  maxSockets: 10,
  maxKeepAliveRequests: 0,
  maxKeepAliveTime: 300000 }

/Users/jvonnieda/Desktop/test/node_modules/elasticsearch/src/lib/log.js:44
      throw new TypeError('Invalid logging output config. Expected either a lo
            ^
TypeError: Invalid logging output config. Expected either a log level, array of log levels, a logger config object, or an array of logger config objects.
    at new Log (/Users/jvonnieda/Desktop/test/node_modules/elasticsearch/src/lib/log.js:44:13)
    at new Transport (/Users/jvonnieda/Desktop/test/node_modules/elasticsearch/src/lib/transport.js:17:27)
    at Object.EsApiClient (/Users/jvonnieda/Desktop/test/node_modules/elasticsearch/src/lib/client.js:49:22)
    at new Client (/Users/jvonnieda/Desktop/test/node_modules/elasticsearch/src/lib/client.js:60:10)
    at Object.<anonymous> (/Users/jvonnieda/Desktop/test/es-config-bug.js:10:11)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Function.Module.runMain (module.js:497:10)

[spalger]

I agree that this is bad. Working on a fix

[jvonniedapn]

Sweet, thanks!

[spalger]

Ok, started implementing this but I've decided against it. It sounds like a good idea on the surface but the configuration is very flexible and can regularly contain objects that shouldn't be cloned (loggers, transports, etc.). The specific objects that shouldn't be cloned couldn't be defined either, since it is completely legal for users to override certain classes and therefore the config they require. Because of this, it makes more sense to say that the config object belongs to the Client once it is sent in. I'd suggest making your config in a function and then passing the result into the client constructor:

function config() {
  return {
    log: "warning"
  };
}

var es1 = new elasticsearch.Client(config());
var es2 = new elasticsearch.Client(config());

[gabegorelick]

The docs should probably highlight this.

[spalger]

@gabegorelick you got it. I added a warning to the top of the configuration section. http://www.elasticsearch.org/guide/en/elasticsearch/client/javascript-api/current/configuration.html

[gabegorelick]

Awesome. Hopefully you won't get any more bug reports about it now.

On Saturday, March 29, 2014, spenceralger notifications@github.com wrote:

@gabegorelick https://github.com/gabegorelick you got it. I added a
warning to the top of the configuration section.
http://www.elasticsearch.org/guide/en/elasticsearch/client/javascript-api/current/configuration.html

Reply to this email directly or view it on GitHubhttps://github.com//issues/33#issuecomment-39013086
.

[julien51]

IMHO, the library itself should duplicate the object passed when initializing to avoid the hassle of doing so to the implementer.

[spalger]

@julien51 I had that opinion originally, but as stated earlier it's not really an option.

[j-norwood-young]

Note that this pattern works:

const config = {
   log: "warning"
}
const Elasticsearch = require("elasticsearch");
const esclient = new Elasticsearch.Client({ ...config });

[jfspencerJN]

For anyone doing unit testing in the NestJS context, freezing the config object in the module definition, prevents this error from firing repeatedly. Then the service can be mocked in unit tests.

imports: [ElasticsearchModule.register(Object.freeze({ host: '' }))],

[mstar-kk]

My solution is:

const elasticsearch = require('elasticsearch');

const clientA = new elasticsearch.Client(R.merge(config.elasticsearch, {}));
const clientB = new elasticsearch.Client(R.merge(config.elasticsearch, {}));

In javascript, object is passed by reference.For the first time construct instance, constructor function will add a variable named __reused to config. This variable will prevent you from using same config twice.