Observations (Catch listener handler exceptions separately from IO exceptions)

[hovi]

Some observations from using this simple and cool library for a few hours:

• I spent a bit of time trying to figure out the correct token to login, manually doing authflows as per twitch documentation, but only after I saw the link in TwirkBuilder class to link, that does this for me. Would be really helpful to put this link into readme :-D
• When my listener method throws exception, your library propagates it as a problem with parsing IRC messages.

[Gikkman]

Oh wow, I can't believe I hadn't linked it in the README. I'll fix that right away.

As for your second point, I am unsure what you mean. Is it for example if you throw a Runtime exception in a listener?

[hovi]

Actually I just looked incorrectly. It more looks like error message Error in handling the incoming Irc Message written to stderr, not another exception.

Example:

    twirk.addIrcListener(object: TwirkListener {
        override fun onAnything(unformatedMessage: String) {
            throw RuntimeException("failing in listener event")
        }
    })

Stacktrace/error message:

Error in handling the incoming Irc Message
failing in listener event
MainKt$main$1.onAnything(Main.kt:33)
	com.gikk.twirk.Twirk.incommingMessage(Twirk.java:471)
	com.gikk.twirk.InputThread.run(InputThread.java:43)

[Gikkman]

Right, right, now I follow.

So, the issue here is that messages are handled asynchronously on a separate thread, and uncaught exceptions that reach the "top" of that separate thread has to be handled somewhere. If I don't catch that exception, the entire thread risk halting, which would cause it to not process any more IRC messages. Just logging if it gets that far is the least disruptive approach I can think of. You can see the code here: https://github.com/Gikkman/Java-Twirk/blob/master/src/main/java/com/gikk/twirk/InputThread.java#L39

I guess an alternative would be to allow the user to supply a custom handler for uncaught exceptions, that could be an alternative approach to just catching the uncaught exception and log to stderr (as a side note, you can supply a custom logging function so it doesn't just print to stderr). But it isn't possible to throw the exception out of the asynchronous handler, since they aren't part of the linear program flow.

[hovi]

Another possibility is to catch exceptions when handling listeners separately so that exception doesn't propagate this high at "handling input". At that point input has been handled, String line is already processed and no IO errors are possible.

My main point here is, that exceptions at IO level, message parsing, listener should be at different levels. Then the client won't receive message about IO/message parser when it's at listener level. Otherwise I understand exception handling and I think it's fine like this imho :)

[Gikkman]

Ah, alright. Thanks for explaining how you view of it. I misunderstood you at first.

And you got a really good point. I never thought of that, to be honest. I should definitely catch exceptions at different levels at you say, so we can distinguish between OP level and listener level exceptions.

I'll rename this issue to something that explains the idea, and keep it in mind when I do some further updates! :)