Behavior of loading request module is undesirable

[rustyconover]

The code to load the request module freshly causes undesirable effects when the request module loads a shared library. Lets review some code from lib/rp.js:

// Load Request freshly - so that users can require an unaltered request instance!
var request = (function () {
    function clearCache() {
        forEach(keys(require.cache), function (key) {
            delete require.cache[key];
        });
    }

    var temp = assign({}, require.cache);
    clearCache();

    var freshRequest = require('request');

    clearCache();
    assign(require.cache, temp);

    return freshRequest;

})();

This code clears the the require.cache so that the request module can be loaded freshly, but if request.js loads a shared library based module with process.dlopen (for instance the kerberos module) that shared library (the c/c++ part) remains in memory, but as far as node.js is concerned it hasn't been loaded. So the second time that node.js attempts to load the module, it throws an error, because the dynamic loader can't load the same symbols twice.

Here is the error:

module.js:460
  return process.dlopen(module, path._makeLong(filename));
                 ^
Error: Module did not self-register.
    at Error (native)
    at Object.Module._extensions..node (module.js:460:18)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:311:12)
    at Module.require (module.js:366:17)
    at require (module.js:385:17)
    at Object.<anonymous> ([redacted]/node_modules/kerberos/lib/kerberos.js:1:78)
    at Module._compile (module.js:435:26)
    at Object.Module._extensions..js (module.js:442:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:311:12)
    at Module.require (module.js:366:17)
    at require (module.js:385:17)
    at Object.<anonymous> ([redacted]/node_modules/kerberos/index.js:2:18)
    at Module._compile (module.js:435:26)
    at Object.Module._extensions..js (module.js:442:10)

Node issue: nodejs/node#5016 may be relevant.

Please consider an alternative behavior that is less error prone or one that is more friendly to modules that include a shared library component.

[analog-nico]

Thanks for bringing that up @rustyconover ! Until either node fixes the referenced issue or the new request architecture becomes available that will allow to provide the promise interface as a plugin, this is a tough puzzle to solve. To still allow using request and request-promise side-by-side currently my best idea is to not clear the whole cache but keep all native modules that request may require in the cache. This would require a little reverse engineering but will be a working interims solution until the new request architecture becomes available. Do you agree or do you have some tips for a cleaner solution?

[analog-nico]

I found out that this issue is not relevant in practice because

1. Request does not require native modules (at least right away) and
2. Request-Promise restores the old cache after Request was required.

Here is the test code that verifies that:

var k = require('kerberos');

var request = require('request');

var rp = require('request-promise'); // ==> Success - verifies point 1

var k2 = require('kerberos'); // ==> Success - verifies point 2

// Clear the cache like Request-Promise does but without restoring it!
var _ = require('lodash');
_.forEach(_.keys(require.cache), function (key) {
    delete require.cache[key];
});

var k3 = require('kerberos'); // ==> Error: Module did not self-register. - ensures the test would fail

Nonetheless, thanks for bringing this up @rustyconover ! I was not aware of that and it could very well have been an issue or may even become one in the future. I will keep an eye on that!