Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error: cannot enable cancellation after promises are in use #319

Closed
mania25 opened this issue Apr 29, 2017 · 13 comments
Closed

Error: cannot enable cancellation after promises are in use #319

mania25 opened this issue Apr 29, 2017 · 13 comments
Labels
Milestone

Comments

@mania25
Copy link

mania25 commented Apr 29, 2017

Hi, i've got error message like this :

9|Grinasia | Error: cannot enable cancellation after promises are in use 9|Grinasia | at Function.Promise.config (/usr/local/www/backend-server/grinasia-backend-new-development/node_modules/bluebird/js/release/debuggability.js:269:19) 9|Grinasia | at Object.<anonymous> (/usr/local/www/backend-server/grinasia-backend-new-development/node_modules/node-telegram-bot-api/src/telegram.js:29:9) 9|Grinasia | at Module._compile (module.js:571:32) 9|Grinasia | at Object.Module._extensions..js (module.js:580:10) 9|Grinasia | at Module.load (module.js:488:32) 9|Grinasia | at tryModuleLoad (module.js:447:12) 9|Grinasia | at Function.Module._load (module.js:439:3) 9|Grinasia | at Module.require (module.js:498:17) 9|Grinasia | at require (internal/module.js:20:19) 9|Grinasia | at Object.<anonymous> (/usr/local/www/backend-server/grinasia-backend-new-development/node_modules/node-telegram-bot-api/index.js:12:20) 9|Grinasia | at Module._compile (module.js:571:32) 9|Grinasia | at Object.Module._extensions..js (module.js:580:10) 9|Grinasia | at Module.load (module.js:488:32) 9|Grinasia | at tryModuleLoad (module.js:447:12) 9|Grinasia | at Function.Module._load (module.js:439:3) 9|Grinasia | at Module.require (module.js:498:17) 9|Grinasia | at require (internal/module.js:20:19) 9|Grinasia | at Object.<anonymous> (/usr/local/www/backend-server/grinasia-backend-new-development/services/telegram.js:1:81) 9|Grinasia | at Module._compile (module.js:571:32) 9|Grinasia | at Object.Module._extensions..js (module.js:580:10) 9|Grinasia | at Module.load (module.js:488:32) 9|Grinasia | at tryModuleLoad (module.js:447:12) 9|Grinasia | at Function.Module._load (module.js:439:3) 9|Grinasia | at Module.require (module.js:498:17) 9|Grinasia | at require (internal/module.js:20:19) 9|Grinasia | at Object.<anonymous> (/usr/local/www/backend-server/grinasia-backend-new-development/routes/flights.js:11:16) 9|Grinasia | at Module._compile (module.js:571:32) 9|Grinasia | at Object.Module._extensions..js (module.js:580:10)

Please help to solve this, thank you

@GochoMugo
Copy link
Collaborator

Do you happen to use promises before loading this library i.e. require("node-telegram-bot-api")?

@dreadlocked
Copy link

I'm receiving exactly the same error message

@GochoMugo
Copy link
Collaborator

@dreadlocked Are you using promises before loading this library?

@fiznool
Copy link

fiznool commented Jun 12, 2017

I'm seeing the same error message, and yes I am using promises before loading the library. Is there a fix available?

@fiznool
Copy link

fiznool commented Jun 12, 2017

OK, so I see. This issue crops up if you are using bluebird in your app and you use a promise before requiring the telegram library. A simple solution is to do the following at the top of your app's entrypoint:

const Promise = require('bluebird');
Promise.config({
  cancellation: true
});

@GochoMugo GochoMugo added bug and removed investigate labels Jun 22, 2017
@GochoMugo
Copy link
Collaborator

I think the library should not configure promises. That should be something done by the host code (read: the library user). Such a fix would break backwards compatibility, thus will need to wait semver-major release.

However, we could allow the users to provide their preferred (and configured) promise library and work with that instead of the current default i.e. bluebird. We could then just drop bluebird and use native promises. *I smell a PR here 😉*

/cc @yagop

@GochoMugo GochoMugo added the TODO label Jun 22, 2017
@fiznool
Copy link

fiznool commented Jun 27, 2017

I also think it's dangerous to opt-in to a configuration parameter such as this. Promise cancellation in bluebird affects the way that timeouts work.

One thing that's bitten me is the changing of how timeouts work when this config is enabled.

This is best explained with a code snippet.

const Promise = require('bluebird');

function prepare() {
  return new Promise((resolve, reject) => {
    setTimeout(() => resolve(), 2000);
  });
}

function build() {
  return prepare().then(() => console.log('it was built'));
}

function run() {
  return build()
    .timeout(1000)
    .catch(Promise.TimeoutError, err => console.log('there was an error'));
}

run();

When running the code above you will see:

there was an error
it was built

If you add the cancellation config, the 'it was built' log is never written. It seems that .timeout will cancel the underlying promise if this config param is set.

Needless to say, this has caused a lot of issues in my app that would be better resolved by not having this config set. 😄

GochoMugo added a commit that referenced this issue Jun 28, 2017
GochoMugo added a commit that referenced this issue Jun 28, 2017
@GochoMugo
Copy link
Collaborator

Update:

  1. Deprecate auto-enabling of cancellation of Promises (commit 997393b)
  2. Allow providing custom Promise constructor/library (commit c2902db)

@GochoMugo GochoMugo removed the TODO label Jun 28, 2017
GochoMugo added a commit that referenced this issue Jul 6, 2017
Side-effects:

  * Deprecate auto-enabling of cancellation of Promises

References:

  * BR: #319
@hems
Copy link

hems commented Jul 29, 2017

Maybe you guys will laugh, but simply requiring "node-telegram-bot-api" at the beggining of the app did the trick for me, i didn't even need to change anything on my app.

ie: app.coffee

P = require 'bluebird'
T = require 'node-telegram-bot-api'

# rest of my app now works perfectly

@setyongr
Copy link

Is your update not yet merged to master? @GochoMugo
Because i still experiencing this issue

@GochoMugo
Copy link
Collaborator

@hems Yes. require()ing the library before using any promises works around this bug.

@setyongr We just deprecated this. Let me deliver a proper fix for this.

GochoMugo added a commit that referenced this issue Aug 25, 2017
Side-effects:

  src/telegram: Allow providing custom Promise constructor

References:

  * BR: #319
@GochoMugo GochoMugo added this to the v1 milestone Aug 25, 2017
GochoMugo added a commit that referenced this issue Aug 25, 2017
Notes:

  The permanent fix introduces backwards-incompatible changes, which
  require a major version bump. Until we release v1,
  environment variable `${NTBA_FIX_319}` can be used to apply
  the fix.

References:

  * BR: #319
@GochoMugo
Copy link
Collaborator

Conclusion:

  • Automatic enabling of Promise cancellation has been deprecated.
  • Temporary fix : load/require() the library, as early as possible, before using any Promises.
  • Permanent fix: set the environment variable NTBA_FIX_319 (to any value). You will have to enable Promise cancellation yourself, if you need it. This fix will be applied by default once we bump major version as it introduces backwards-incompatible changes.
  • This issue has been locked to ensure that this message reaches anyone who might stumble upon it. If necessary, please create a new issue.
  • This issue will be closed once we hit v1.

Repository owner locked and limited conversation to collaborators Aug 25, 2017
@danielperez9430
Copy link
Collaborator

Fix on the next release. Remove bluebird and use native promises

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

7 participants