search

moquette-io / moquette

Java MQTT lightweight broker
http://moquette-io.github.io/moquette/
Apache License 2.0
2.3k stars 818 forks source link

Moquette broker kills parent application if embedded and broker port already used #641 #646

Closed sbaranov-iqmessenger closed 2 years ago

sbaranov-iqmessenger commented 2 years ago

Release notes

Avoid Server.startServer to exit the process it port is bound, just raise an error (#646)

#641 just there was removed System.exit(1)

andsel commented 2 years ago

The io.moquette.broker.Server class is also used in bat and sh scripts used by the tar.gz distribution of Moquette. To complete this PR we should create another class (maybe io.moquette.broker.Launcher that simply wrap Server and catch for the exceptions, because in case of bind error, it should exit with error code without ugly traces in the console.

sbaranov-iqmessenger commented 2 years ago

The io.moquette.broker.Server class is also used in bat and sh scripts used by the tar.gz distribution of Moquette. To complete this PR we should create another class (maybe io.moquette.broker.Launcher that simply wrap Server and catch for the exceptions, because in case of bind error, it should exit with error code without ugly traces in the console.

I didn't change current exception "culture" and don't throw BindException to not add additional exceptions in method signatures. What do you think ?

sbaranov-iqmessenger commented 2 years ago

We are almost there, please integrate the suggestion and we are good to merge.

I've committed what you suggested but is there any benefit of containing human readable "Cannot bind..." because in Server.main I don't log exception. I just call System.exit(1) without any logging

andsel commented 2 years ago

is there any benefit of containing human readable "Cannot bind..."

I think it's useful in the code that use that method, incorporating it as a library, maybe it want to log the cause of the error, if we don't throw a specific error excpetion.

sbaranov-iqmessenger commented 2 years ago

is there any benefit of containing human readable "Cannot bind..."

I think it's useful in the code that use that method, incorporating it as a library, maybe it want to log the cause of the error, if we don't throw a specific error excpetion.

oh, yeah, you are right. I got it

andsel commented 2 years ago

Thanks a lot @sbaranov-iqmessenger for contributing! Warm welcome