getmoro/moro

Bluebird warnings

Closed this issue · 2 comments

Great project! Just installed and am seeing soft warnings in from Bluebird promises on most commands. Code works as inspected but warnings ruin the nice cli output.

Cause seems to be "missing" returns on chains leading to "runaway" promises:

Bluebird Docs — Warning: a promise was created in a handler but was not returned from it.

MacOS Sierra: 10.12.16
Nodejs: 6.11.2
Bash: 4.4.12
Moro: v3.2.0 (latest) from npm

Examples:

moro or moro hi [time]

$ moro
$ (node:8633) Warning: a promise was created in a handler at /Users/aaron/.node-global-modules/lib/node_modules/moro/lib/commands.js:55:10 but was not returned from it, see http://goo.gl/rRqMUw
    at Function.Promise.attempt.Promise.try (/Users/aaron/.node-global-modules/lib/node_modules/moro/node_modules/bluebird/js/release/method.js:29:9)

moro search [term]

$ moro search first
$ (node:9710) Warning: a promise was created in a handler at /Users/aaron/.node-global-modules/lib/node_modules/moro/lib/commands.js:236:10 but was not returned from it, see http://goo.gl/rRqMUw
    at Function.Promise.attempt.Promise.try (/Users/aaron/.node-global-modules/lib/node_modules/moro/node_modules/bluebird/js/release/method.js:29:9)

Have removed the warnings locally by adding return's in all the suitable places I can see. For example to solve the first warning I've updated lib/commands.js lines 53–60 from:

// update database
db
  .updateDatabase(payload, db.knex)
  .then(() => {
    spinner.succeed(`You clocked in at: ${start}\n\n`).start()
    helpers.shouldWorkUntil(start, config)
    spinner.info(constants.TEXT.clockOutTip)
})

to

// update database
return db
  .updateDatabase(payload, db.knex)
  .then(() => {
    spinner.succeed(`You clocked in at: ${start}\n\n`).start()
    helpers.shouldWorkUntil(start, config)
    spinner.info(constants.TEXT.clockOutTip)

    return
})

Anyone else seeing this? Or am I missing something specific on my end?

Thanks

Hi @aaronbates ,

I just published moro 3.2.1, It should be fixed now, please update your moro:
npm update -g moro

By the way, you were probably the only one seeing the warnings because of having an env variable like this:
BLUEBIRD_WARNINGS=1
source: http://bluebirdjs.com/docs/api/promise.config.html

Thanks for contributing by creating a nice and clear issue ❤️

Ah, good spot on the warnings config and thanks for the commit @omidfi.

My NODE_ENV is set to development — that does seem a little opinionated by them, especially if the warnings are supposed to be useful... I'll build a function into my bash config to toggle the warnings locally better.

I've noticed that moro search still gives a warning, would you mind adding a return to line 225 in lib/commands.js? No rush! Only if you get chance — I realise this is more of a "quality of life" improvement.

Thanks again!

219 const search = (args, options, logger) => {
220   let searchTerm = args.term.join(' ')
221
222   configManager
223     .configLoaded()
224     .then(config => {
225       db
226       .getSearchTerm(searchTerm, db.knex, config)