Bugfix - Handling blocked users gracefully when reacting to their messages

[interacsion]

warped the line that reacts to the message in a try catch block. see file changes.
resolves #406.

[interacsion]

Why complete

can't catch queue(), afaik.

[interacsion]

iirc, queue() throws exceptions on the executing thread, thus you can't catch them on the calling thread.
correct me if I am wrong

[Tais993]

queue is async indeed, so a try-catch doesn't work.
Instead you have to use the consumers, take a look at the documentation here.

[interacsion]

.queue(ignored -> {},
exception -> {
    if (exception instanceof ErrorResponseException && ((ErrorResponseException) exception).getErrorResponse() == ErrorResponse.REACTION_BLOCKED) {
        return;
    }
    exception.printStackTrace();
});

is this better?

[Zabuzard]

u have to use our logging system, not e.printStackTrace. so at least logger.error("...", e)

[interacsion]

The best thing to do is to throw it, probably, but how do I do it from inside the function? 🤔

I think staying with .complete() and try catch is the best solution. I think we shouldn't handle other exceptions here.

[Zabuzard]

The best thing to do is to throw it, probably, but how do I do it from inside the function? 🤔

Then you would throw from within the JDA context where the lambda is executed, which is detached from our flow - not good.

If you stick with queue, you should log the error using the logger.

If you stick with complete, try-catch is fine. That means that the call is blocking though and not async. Which means that you could potentially block one of our bot-core-system threads for a long time. We only have a few of these, so if there are many reactions going on and they, for whatever reason, take long to complete, that can block the whole bot and stuff like /tag foo can not be executed anymore and will die, cause they will want to use the same threads, which are all blocked by the slow reaction.

And all of that just because you want to throw an exception, you do not even have business logic or anything that requires a blocking call. Thats not really worth it.

tldr, dont block our threads, use queue.

(in this particular case it would actually be safe though, since as of today, BotCore doesnt manage EventListeners itself but just forward them to JDA directly - so they dont share an executorservice with slash commands)

[interacsion]

that should do it.

[interacsion]

When it logs an exception the stack trace shows this.

is that supposed to happen? it doesn't happen if I don't use any error handler for the queue().

[Tais993]

I don't know or it should, please give the full stacktrace

[interacsion]

Here ya go
https://gist.github.com/MaiTheLord/90c678e4948d733c7a4edcb7a48dd715

[Zabuzard]

Sounds like you logged the error now. Didnt you want to ignore it and only log unknown failures? Whats the code that produced this?

[interacsion]

I commented the part that ignores the error, for the sake of seeing a stacktrace, as I couldn't think of any other way to make it fail.

[interacsion]

I found another way to make it fail.
https://gist.github.com/MaiTheLord/99335c4206d4f81da055df4fbb5e6deb
produced by latest code, 8d554a3, and revoking the Add Reactions permission

[Zabuzard]

Not sure what your question or confusion is, to be honest. This is the expected behavior - it logs the unknown error. Looks good to me 👍

[interacsion]

I wasn't sure about the caused by ContextException.

when I look at it now, seems like its JDA's way to handle async exceptions.

[Tais993]

When it logs an exception the stack trace shows this.

ah yeah, that's just the exception, scroll up and you see the cause.

I found another way to make it fail.
https://gist.github.com/MaiTheLord/99335c4206d4f81da055df4fbb5e6deb
produced by latest code, 8d554a3, and revoking the Add Reactions permission

There's a ton of ways to make it fail, but we shouldn't do anything with this.
It logs the error, the bot owner can add permissions, not our issue.
Just keep it like this

[interacsion]

Zabuzard, can you review please?

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication