robtweed / ewd-qoper8

Node.js Message Queue and Multi-Process Manager
24 stars 5 forks source link

process.argv[2] !== 'undefined') #4

Open killmenot opened 7 years ago

killmenot commented 7 years ago

Hi @robtweed

Please take a look: https://github.com/robtweed/ewd-qoper8/blob/master/lib/worker/proto/init.js#L36

I believe that line 36 should be

if (process.argv[2] && typeof process.argv[2] !== 'undefined') {

However, I think that


if (typeof process.argv[2] !== 'undefined') {
``
also will work
robtweed commented 7 years ago

In fact I believe my code is correct. If no worker module is defined in the startup callback, then there will be an actual value of 'undefined' in the process argv array. If you look at the various tests, some don't define a module - in which case the default worker handers are used. However test5.js, test6.js etc define a custom module. Try looking at this line in question when you run, eg test4.js versus test5.js and see what ends up in the process.argv[2] array element

killmenot commented 7 years ago

@robtweed

Please take a look at the following screenshot:

screenshot 2017-08-10 15 55 47

The content of the test.js file is pretty simple, just console.log(process.argv);

I believe we're talking about line 42 and line 64 here.

If no worker module defined, there is no value for process.argv[2]. In this case the correct condition should be

typeof process.argv[2] !== 'undefined'

because

process.argv[2] && process.argv[2] !== 'undefined'

do a little bit different things:

  1. check if process.argv[2] exists (any non empty value)
  2. check if process.argv[2] is not equal 'undefined' (compare strings)

That's why I believe typeof process.argv[2] !== 'undefined' is more correct rule here

killmenot commented 7 years ago

Other questions:

  1. What if module path contain dot? example: path/to/my.module.file?
  2. Do we need support of relative paths?

At the current moment module can be loaded

I created examples repository - https://github.com/killmenot/ewd-qoper8-examples/blob/master/examples/test5.js#L38

However, when I run this test file I got the following error:

process.argv[2] = examples/modules/example-worker-module
workerModule: examples/modules/example-worker-module; undefined
module.js:487
    throw err;
    ^

Error: Cannot find module 'examples/modules/example-worker-module'
    at Function.Module._resolveFilename (module.js:485:15)
    at Function.Module._load (module.js:437:25)
    at Module.require (module.js:513:17)
    at require (internal/module.js:11:18)
    at workerProcess.init (/Users/killmenot/projects/github/killmenot/ewd-qoper8-examples/node_modules/ewd-qoper8/lib/worker/proto/init.js:50:25)
    at Object.<anonymous> (/Users/killmenot/projects/github/killmenot/ewd-qoper8-examples/node_modules/ewd-qoper8-worker.js:5:8)
    at Module._compile (module.js:569:30)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:503:32)
    at tryModuleLoad (module.js:466:12)

I think if we improve require calls here https://github.com/robtweed/ewd-qoper8/blob/master/lib/worker/proto/init.js#L46-L50 we may allow to load scripts from process.cwd folder at least.

robtweed commented 7 years ago

If you can think of a backward-compatible way to load module other than by absolute path or relative to the node_modules directory, then yes - good idea. But it must be backward-compatible with how it works currently

robtweed commented 7 years ago

Try running the test named benchmark.js, eg:

node benchmark 2 1000 100 100

If you trap the process.argv array in init() you'll see:

process.argv = ["/home/rtweed/.nvm/versions/node/v8.3.0/bin/node","/home/rtweed/qewd/node_modules/ewd-qoper8-worker.js","undefined"]

The code in init() is correct. Change it to what you propose and the benchmark test fails