Too broad warning on missing package.json while feature initialization

[hawkeye64]

I'm submitting a...

• Bug report
• Feature request
• Question

Current behavior

running db-migrate up gives the following error:

$ db-migrate -v
[WARN] TypeError: Cannot convert undefined or null to object
    at Function.assign (<anonymous>)
    at loadPluginList (/home/jeff/.nvm/versions/node/v8.15.1/lib/node_modules/db-migrate/index.js:14:20)
    at loadPlugins (/home/jeff/.nvm/versions/node/v8.15.1/lib/node_modules/db-migrate/index.js:24:17)
    at Object.module.exports.getInstance (/home/jeff/.nvm/versions/node/v8.15.1/lib/node_modules/db-migrate/index.js:56:51)
    at /home/jeff/.nvm/versions/node/v8.15.1/lib/node_modules/db-migrate/bin/db-migrate:32:25
    at /home/jeff/.nvm/versions/node/v8.15.1/lib/node_modules/db-migrate/node_modules/resolve/lib/async.js:81:17
    at processDirs (/home/jeff/.nvm/versions/node/v8.15.1/lib/node_modules/db-migrate/node_modules/resolve/lib/async.js:219:39)
    at isdir (/home/jeff/.nvm/versions/node/v8.15.1/lib/node_modules/db-migrate/node_modules/resolve/lib/async.js:226:32)
    at /home/jeff/.nvm/versions/node/v8.15.1/lib/node_modules/db-migrate/node_modules/resolve/lib/async.js:23:69
    at FSReqWrap.oncomplete (fs.js:152:21)

Expected behavior

no error

Minimal reproduction of the problem with instructions

The migrations are SQL format.

database.json

{
  "defaultEnv": "dev",
  "dev": {
    "driver": "pg",
    "user": "*****",
    "password": "*****",
    "host": "localhost",
    "database": "*****",
    "schema": "*****"
  },
  "sql-file": true
}

Environment

db-migrate version: 0.11.5
plugins with versions: none
db-migrate driver with versions: db-migrate-pg@0.5.1

Additional information:
- Node version: 8.15.1
- Platform:  Linux

Others:
Rolled back to v0.11.3 and getting no error

[wzrdtales]

mhm this is strange. Are you sure you can reproduce this error?

I tried and I can not and also looking at the code it does not make sense. There were no changes on the plugin logic in those versions (0.11.3...0.11.5)

[wzrdtales]

Ok now I am understanding. So this is not a bug (the specification of this ticket is not correct that would be rather a feature request to improve on the logging). I first thought you would have run into a bug and db-migrate is not working anymore, but that is not the case. It displays the error to you, you should have got before. And in this case this error most certainly means, there is no package.json in your current working directory.

Indeed, the only thing added is a log, when something wents wrong with the plugin initialization. Which can be improved since it is not handling the issue but just logging the error.

node-db-migrate/index.js

Line 58 in 108087d

log.warn(ex);

Relabling it for refactoring

Please try in the future to be more specific and provide enough information and choose a more useful title for the issue.

[hawkeye64]

There is in fact a package.json in that folder as well as our "migrations" folder. Upgrading to db-migrate@ 0.11.5 and then running db-migrate up in this folder, resulted in the error. I downgraded to db-migrate@0.11.3, re-ran db-migrate up and it worked. All without leaving said folder. I consider it a bug. We have no plugins. It crashed in the loadPluginList function.

You're saying a package.json is required?
Ours looks like this:

{
  "name": "ivt-database",
  "description": "IVT Database migrations.",
  "author": "**********",
  "version": "5.0.0",
  "private": true,
  "false": {}
}

Is there something db-migrate is looking for in the package.json?

[wzrdtales]

No it is not required and neither it should crash. Actually it can't crash, so I guess you misinterpreted it.

node-db-migrate/index.js

Lines 64 to 68 in 9334ee2

	try {
	if (!options.noPlugins) plugins = loadPlugins(options);
	} catch (ex) {
	log.warn(ex);
	}

As you can see the whole plugin block is encapsulated in a try catch block, and log warn does really only print a warning. nothing more.

[hawkeye64]

It crashes here:

plugins = Object.assign(plugins.dependencies, plugins.devDependencies);

I guess the issue would be to see why it's getting in this code anyway, with no plugins defined and why the try/catch appears to not be working.

[hawkeye64]

In this code:

module.exports.getInstance = function (isModule, options, callback) {
  delete require.cache[require.resolve('./api.js')];
  delete require.cache[require.resolve('optimist')];
  var Mod = require('./api.js');
  var plugins = {};

  try {
    if (!options || !options.noPlugins) plugins = loadPlugins();
  } catch (ex) {
    log.warn(ex);
  }

  if (options && options.plugins) {
    plugins = Object.assign(plugins, options.plugins);
  }

  return new Mod(plugins, isModule, options, callback);
};

and the reason it's looking for for plugins is options is undefined which cause this code:

    if (!options || !options.noPlugins) plugins = loadPlugins();

to be true[!options]...

[wzrdtales]

db-migrate does not crash. It just prints a warning.

bin/db-migrate --version
[WARN] TypeError: Cannot convert undefined or null to object
    at Function.assign (<anonymous>)
    at loadPluginList (/home/tobi/serious_projects/db-migrate/node-db-migrate/index.js:14:20)
    at loadPlugins (/home/tobi/serious_projects/db-migrate/node-db-migrate/index.js:24:17)
    at Object.module.exports.getInstance (/home/tobi/serious_projects/db-migrate/node-db-migrate/index.js:56:51)
    at /home/tobi/serious_projects/db-migrate/node-db-migrate/bin/db-migrate:32:25
    at /home/tobi/serious_projects/db-migrate/node-db-migrate/node_modules/resolve/lib/async.js:52:13
    at processDirs (/home/tobi/serious_projects/db-migrate/node-db-migrate/node_modules/resolve/lib/async.js:184:39)
    at ondir (/home/tobi/serious_projects/db-migrate/node-db-migrate/node_modules/resolve/lib/async.js:199:13)
    at load (/home/tobi/serious_projects/db-migrate/node-db-migrate/node_modules/resolve/lib/async.js:82:43)
    at onex (/home/tobi/serious_projects/db-migrate/node-db-migrate/node_modules/resolve/lib/async.js:107:17)
0.11.5
echo $?
0

You can see the exit code, and notice that further functionality is being executed. So the try catch just works as expected. However, taking a look, actually without a package.json it would fail differently. Since in v0.11.5 it still directly reads, without checking for a package.json before. There is a patch for that on the master already, which I would need to backport (and maybe adjust). In this case, the completely missing properties are not being handled and checked, which is the reason for the warning. So this is an issue to its own.

[hawkeye64]

ok, fair enough. For now, I can roll back and use previous version. Thanks for your time.

[hawkeye64]

Sorry, didn't mean to close.

[wzrdtales]

no worries, I will close and let you know as soon as that got taken care of.

[hawkeye64]

Thanks

[wzrdtales]

0.11.6 now only logs in verbose mode and adds handling for non existent dependency properties.

[hawkeye64]

Excellent! Look forward to testing. Thanks for your hard work. It's appreciated.

[hawkeye64]

Works great!
Even db-migrate --version works when package.json does not exist.