Add implementation for pm-require

lib/sandbox/execute.js

@@ -9,6 +9,7 @@ const _ = require('lodash'),
     PostmanTimers = require('./timers'),
     PostmanAPI = require('./pmapi'),
     PostmanCookieStore = require('./cookie-store'),
+    PostmanRequireStore = require('./pm-require'),
 
     EXECUTION_RESULT_EVENT_BASE = 'execution.result.',
     EXECUTION_REQUEST_EVENT_BASE = 'execution.request.',
@@ -227,9 +228,9 @@ module.exports = function (bridge, glob) {
                         timers.terminate(null);
                     },
                     dispatchAssertions,
-                    options.requireFileResolver,
-                    scope,
                     new PostmanCookieStore(id, bridge, timers),
+                    new PostmanRequireStore(options.requireFileResolver),
+                    scope,
                     {
                         disabledAPIs: initializationOptions.disabledAPIs
                     })

lib/sandbox/pm-require.js

@@ -0,0 +1,21 @@
+class PostmanRequireStore {
+    constructor (fileCache) {
+        this.fileCache = fileCache || {};
+    }
+
+    getResolvedPath (path) {
+        if (this.hasFile(path)) {
+            return path;
+        }
+    }
+
+    hasFile (path) {
+        return Boolean(this.fileCache[path]);
+    }
+
+    getFileData (path) {
+        return this.hasFile(path) && this.fileCache[path].data;
+    }
+}
+
+module.exports = PostmanRequireStore;

lib/sandbox/pmapi.js

@@ -7,6 +7,12 @@ const _ = require('lodash'),
     PostmanCookieList = sdk.CookieList,
     chai = require('chai'),
 
+    MODULE_KEY = '__module_obj', // why not use `module`?
+    MODULE_WRAPPER = [
+        '(function (exports, module) {\n',
+        `\n})(${MODULE_KEY}.exports, ${MODULE_KEY});`
+    ],
+
     /**
      * Use this function to assign readonly properties to an object
      *
@@ -50,7 +56,7 @@ const _ = require('lodash'),
  * @param {Object} [options] - options
  * @param {Array.<String>} [options.disabledAPIs] - list of disabled APIs
  */
-function Postman (execution, onRequest, onSkipRequest, onAssertion, requireFileResolver, scope, cookieStore, options = {}) {
+function Postman (execution, onRequest, onSkipRequest, onAssertion, cookieStore, requireStore, scope,  options = {}) {
     // @todo - ensure runtime passes data in a scope format
     let iterationData = new VariableScope();
 
@@ -294,39 +300,55 @@ function Postman (execution, onRequest, onSkipRequest, onAssertion, requireFileR
         },
 
         require: function require (name) {
-            var file = (requireFileResolver || {})[name];
+            const path = requireStore.getResolvedPath(name);
 
-            if (!file || !file.data) {
-                throw new Error(`Cannot find module '${name}'`);
+            this.cache = this.cache || {};
+
+            // Always use the resolved path as the ID of the module. This
+            // ensures that relative paths are handled correctly.
+            if (this.cache[path]) {
+                return this.cache[path].exports;
             }
 
-            const codeToExecute = [
-                '(function (module, exports) {',
-                file.data,
-                '})(__module_obj, __module_obj.exports);'
-            ].join('\n');
+            const file = path && requireStore.getFileData(path);
+
+            if (!file) {
+                // Error should contain the name exactly as the user specified.
+                throw new Error(`Cannot find module '${name}'`);
+            }
 
             const moduleObj = {
+                id: path,
                 exports: {}
             };
 
-            // when is this cache cleaned up? Memory leak???
-            this.__cache = this.__cache || {};
-
-            if (this.__cache[name]) {
-                return this.__cache[name].exports;
-            }
+            // Add to cache before executing. This ensures that any dependency
+            // that tries to import it's parent/ancestor gets the cached
+            // version and not end up in infinite loop.
+            this.cache[moduleObj.id] = moduleObj;
 
-            this.__cache[name] = moduleObj;
+            const wrappedModule = MODULE_WRAPPER[0] + file + MODULE_WRAPPER[1];
 
-            scope.import({ __module_obj: moduleObj });
+            scope.import({
+                [MODULE_KEY]: moduleObj
+            });
 
-            scope.exec(codeToExecute, { async: true, resetLocals: false }, (err) => {
-                scope.unset('__module_obj');
-                throw err;
+            // Note: We're executing the code in the same scope as the one
+            // which called the `pm.require` function. This is because we want
+            // to share the global scope across all the required modules. Any
+            // locals are available inside the required modules and any locals
+            // created inside the required modules are available to the parent.
+            //
+            // Why `async` = true?
+            //   - We want to allow execution of async code like setTimeout etc.
+            scope.exec(wrappedModule, true, (err) => {
+                // Bubble up the error to be caught as execution error
+                if (err) {
+                    throw err;
+                }
             });
 
-            scope.unset('__module_obj');
+            scope.unset(MODULE_KEY);
 
             return moduleObj.exports;
         }

test/unit/sandbox-libraries/pm-require.test.js

@@ -88,6 +88,7 @@ describe.only('sandbox library - pm.require api', function () {
             var assert = require('assert');
             try {
                 pm.require('mod1');
+                throw new Error('expected to throw');
             }
             catch (e) {
                 assert.strictEqual(e.message, "my error");
@@ -96,14 +97,13 @@ describe.only('sandbox library - pm.require api', function () {
             context: sampleContextData,
             requireFileResolver: {
                 mod1: {
+                    name: 'mod1',
                     data: 'throw new Error("my error");'
                 }
             }
         }, done);
     });
 
-    // FIXME: the globals get set to `undefined` in the `.exec` function of
-    // uniscope.
     it('should allow required files to access globals', function (done) {
         context.execute(`
             var assert = require('assert');
@@ -256,44 +256,23 @@ describe.only('sandbox library - pm.require api', function () {
         }, done);
     });
 
-    it.skip('should not leak memory', function (done) {
-        async.timesSeries(10, function (n, next) {
-            context.execute(`
-                var assert = require('assert');
-                var mod1 = pm.require('mod1');
-                assert.strictEqual(mod1.a, 123);
-            `, {
-                context: sampleContextData,
-                requireFileResolver: {
-                    mod1: {
-                        data: `
-                            module.exports = { a: Math.random().toString(36).substr(2, 9).repeat(20000000) };
-                        `
-                    }
-                }
-            }, function () {
-                console.log('iteration done', n);
-                next();
-            });
-        }, done);
-    });
-
-    it('should not have access to moduleObj', function (done) {
+    // TODO: fixit
+    it.skip('should not have access to __module_obj', function (done) {
         context.execute(`
             var assert = require('assert');
             try {
-                pm.require('mod1');
+                const val = pm.require('mod1');
+                throw new Error('should not reach here');
             }
             catch (e) {
-                assert.strictEqual(e.message, "moduleObj is not defined");
+                assert.strictEqual(e.message, "__module_obj is not defined");
             }
         `, {
             context: sampleContextData,
             requireFileResolver: {
                 mod1: {
                     data: `
-                        module.exports = moduleObj; // moduleObj should not be defined
-                        throw new Error('should not reach here');
+                        module.exports = __module_obj; // __module_obj should not be defined
                     `
                 }
             }
@@ -303,7 +282,7 @@ describe.only('sandbox library - pm.require api', function () {
     it('should allow async operations', function (done) {
         const errorSpy = sinon.stub();
 
-        context.once('execution.error', errorSpy);
+        context.on('execution.error', errorSpy);
         context.execute(`
             const assert = require('assert');
             const mod1 = pm.require('mod1');
@@ -336,4 +315,149 @@ describe.only('sandbox library - pm.require api', function () {
             done();
         });
     });
+
+    it('should catch errors in async code', function (done) {
+        const errorSpy = sinon.stub();
+
+        context.on('execution.error', errorSpy);
+        context.execute(`
+            pm.require('mod1');
+        `, {
+            context: sampleContextData,
+            requireFileResolver: {
+                mod1: {
+                    data: `
+                        setTimeout(function () {
+                            pm.require('mod2');
+                        }, 10);
+                    `
+                },
+                mod2: {
+                    data: `
+                        setTimeout(function () {
+                            throw new Error('my async error');
+                        }, 10);
+                    `
+                }
+            }
+        }, function (err) {
+            if (err) {
+                return done(err);
+            }
+
+            expect(errorSpy, 'there was no error in the script').to.have.been.calledOnce;
+            expect(errorSpy.firstCall.args[1]).to.have.property('message', 'my async error');
+            done();
+        });
+    });
+
+    it('should keep the references for the locals instead of values', function (done) {
+        const errorSpy = sinon.stub();
+
+        context.on('execution.error', errorSpy);
+        context.execute(`
+            const assert = require('assert');
+
+            a = 1;
+            b = 1;
+            obj = {
+                a: 1,
+                b: 1
+            };
+
+            pm.require('mod1');
+            pm.require('mod2');
+
+            assert(a === 2);
+            assert(obj.a === 2);
+
+            setTimeout(function () {
+                assert(b === 2);
+                assert(obj.b === 2);
+            }, 3);
+        `, {
+            context: sampleContextData,
+            requireFileResolver: {
+                mod1: {
+                    data: `
+                        const assert = require('assert');
+
+                        assert(a === 1);
+                        assert(obj.a === 1);
+
+                        a = 2;
+                        obj.a = 2;
+
+                        setTimeout(function () {
+                            assert(b === 1);
+                            assert(obj.b === 1);
+
+                            b = 2;
+                            obj.b = 2;
+                        }, 1);
+                    `
+                },
+                mod2: {
+                    data: `
+                        const assert = require('assert');
+
+                        assert.strictEqual(obj.a, 2, 'sync variables by reference not updated');
+                        assert.strictEqual(a, 2, 'sync variables by value not updated');
+
+                        setTimeout(function () {
+                            assert.strictEqual(obj.b, 2, 'async variables by reference not updated');
+                            assert.strictEqual(b, 2, 'async variables by value not updated');
+                        }, 2);
+                    `
+                }
+            }
+        }, function (err) {
+            if (err) {
+                return done(err);
+            }
+
+            if (errorSpy.firstCall) {
+                console.log('error:', errorSpy.firstCall.args[1].message);
+            }
+            expect(errorSpy).to.not.have.been.called;
+            done();
+        });
+    });
+
+    it('should make "module" available in the required file', function (done) {
+        context.execute(`
+            pm.require('mod1');
+        `, {
+            context: sampleContextData,
+            requireFileResolver: {
+                mod1: {
+                    data: `
+                        var assert = require('assert');
+                        assert.ok(module);
+                        assert.ok(module.exports);
+                        assert.strictEqual(module.exports, exports);
+                        assert.strictEqual(module.id, 'mod1');
+                    `
+                }
+            }
+        }, done);
+    });
+
+    it('should not have access to cache', function (done) {
+        context.execute(`
+            var assert = require('assert');
+            var mod1 = pm.require('mod1');
+
+            assert.strictEqual(pm.require.cache, undefined);
+        `, {
+            context: sampleContextData,
+            requireFileResolver: {
+                mod1: {
+                    data: `
+                        module.exports = { a: 123 };
+                    `
+                }
+            }
+        }, done);
+    });
 });