Skip to content
This repository has been archived by the owner on Jul 3, 2021. It is now read-only.

Add error emitter #90

Open
0xVasconcelos opened this issue Jul 24, 2015 · 9 comments
Open

Add error emitter #90

0xVasconcelos opened this issue Jul 24, 2015 · 9 comments

Comments

@0xVasconcelos
Copy link

Add error emitters to manually handle.

ex: bot.on('error', function(err){});

Especially if fails to connect, throw a error on connection fail.

Only log to app error logger is not flexible.

@0xVasconcelos 0xVasconcelos changed the title Add error's emitters Add error emitters Jul 24, 2015
@0xVasconcelos 0xVasconcelos changed the title Add error emitters Add error emitter Jul 24, 2015
@Talos51
Copy link
Contributor

Talos51 commented Jul 24, 2015

Hi ! Yes it could be great to add an error listener, but I rather think you can handle your code, for now, with

try{
    ... your code ...
} 
catch(err){
    ...handle err here ...
}

Errors aren't just logged insofar as they can crash an app, with a try{] catch(){], you can receive the triggered error and do what you want with it.
It could be a welcomed feature for V4 but since NodeJS is written like this, every fatal error will lead to a process kill by NodeJS so... ( I'm not a NodeJS expert on that point btw )

Thanks.

@0xVasconcelos
Copy link
Author

I already tried try and catch and won't work, the code is using process.exit().

I forked, if I have some progress i'll make a pull request.

Thanks!

@thedark1337
Copy link
Member

I was going to work on throwing errors or using callbacks, depending on if a callback is being used or not in version 4.0 but if you could contribute your code as well that'd be great 👍

@0xVasconcelos
Copy link
Author

Yes, I don't have so much time to work on it, but I'll try.
But, you can make a option on instance call to throw errors or just log to app error logger, will be awesome.
I'm trying to implement your API in a cool project and I need to instantiate your API several times, so without error throwing is impossible to implement.

@thedark1337
Copy link
Member

I've made it so it returns the error in async mode in a callback and throws an error in sync mode in the latest version. Would you still like me to add the option to emit errors instead as an option?

@johnRivs
Copy link

So does that mean, we can listen for error when the bot fails because plugdj is in maintenance mode, and responde accordingly without the node app crashing? :O

That'd save my life.

@thedark1337
Copy link
Member

Plug does send a message to everyone ( a system message) for when maintenance is about to begin, i can add in that in the next release and you can listen to it and handle it your own way... ex


bot.on(bot.PLUG_MAINT, function(data) {
 // handle maintenance here.
 });

Not currently Implemented but will be soonish.

Currently what is implemented is better error handling for Sync and offloading most errors for async to the developer of the bot..

@johnRivs
Copy link

johnRivs commented Sep 1, 2015

The maintenance example you showed seems cool.

I'm gonna have a look around the error thing, cuz I'm done dealing with unlistened errors.

@thedark1337
Copy link
Member

@johnRivs I've added a warning when plug alerts there will be a maintenance mode soon and the example code i gave should work (its actually MAINT_MODE though) in the latest version 👍

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

No branches or pull requests

5 participants