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

DeprecationWarning: Unhandled promise rejections are deprecated. #201

Closed
romanzipp opened this issue Jan 7, 2017 · 7 comments
Closed

DeprecationWarning: Unhandled promise rejections are deprecated. #201

romanzipp opened this issue Jan 7, 2017 · 7 comments

Comments

@romanzipp
Copy link

romanzipp commented Jan 7, 2017

Actual behaviour:

Unhandled promise rejections are deprecated.

Fix:

client.js Line 984

// Send command to server or channel..
client.prototype._sendCommand = function _sendCommand(delay, channel, command, fn) {
    // Race promise against delay..
    return new Promise((resolve, reject) => {
        _.promiseDelay(delay).then(() => { reject("No response from Twitch."); });
        // (...)
        // Disconnected from server..
        else { reject("Not connected to server."); }
    }).catch(() => {});
};

Error log:

(node:26041) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): No response from Twitch.
(node:26041) DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Server configuration

  • Operating system: Debian 8.6
  • Node version (if applicable): 7.4
  • NPM version (if applicable): 4.0.5
  • tmi.js version: 1.1.2
@Schmoopiie
Copy link
Member

Will look into it, thanks!

@zoton2
Copy link

zoton2 commented Jan 8, 2017

I was going to post about this ages ago but never got around to it, noticed this when I upgraded my Node version (to v6) and when I did this my bot would just randomly crash every so often, seemingly randomly. The only errors that were logged were in relation to unhandled promises, so I thought it might be those. Not sure if it was the actual issue, but it still needs fixing of course.

@AlcaDesign
Copy link
Member

AlcaDesign commented Jan 8, 2017

Imagine this was synchronous code throwing an error. You must catch the Promise rejection. Having the catch in the library doesn't make the rejection useful and defeats the purpose.

Without the internal catch:

client._sendCommand(...)
// This will only call if it resolves and then skip the catch to the next
// `then`, assuming `then` doesn't throw:
.then()
// It will skip the `then` if it rejects and come to here:
.catch();

With the internal catch:

client._sendCommand(...)
// This will always be called:
.then()
// This will never be called unless the previous `then` throws:
.catch();

And also the rejection message is now thrown away, so what was the point?

(Unless there's something I'm missing.)

@romanzipp
Copy link
Author

Still open

@Schmoopiie Schmoopiie added this to the 2.0.0 milestone Apr 15, 2017
@Schmoopiie
Copy link
Member

This is something that is going to be fixed in 2.0.0 only. As of now, as written in our readme file and documentation, the module supports Node 4.x.

@Schmoopiie
Copy link
Member

Update: Might be fixed in v1.2.2, but I need reviews for #230.

@erezefrat
Copy link

Is this fix still coming?

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

No branches or pull requests

5 participants