kordlib / kord

Idiomatic Kotlin Wrapper for The Discord API
MIT License
929 stars 81 forks source link

More information in an stack trace / preconditions #949

Closed NikDeKur closed 5 months ago

NikDeKur commented 5 months ago

During development I encountered a problem caused by my code, which unintentionally sent very long text to audit log reason and to DM user. In all two cases the stack trace was unclear and only led to internal gateway methods, which did not help to solve the error or understand its cause.

In the case with audit log reason the message in stack trace did not give any useful information: “REST request returned an error: 400”, and the case with private messages was recognized by an error from discord “REST request returned an error: 400 Invalid Form Body {”content“:{”_errors“:[{”code“: ‘BASE_TYPE_MAX_LENGTH’, ‘message’: ‘Must be 4000 or less in length.’}]}}”.

I think it is also important to note that stack trace does not mention the function that created the request, which makes it very difficult to detect the error. I had to put a ton of logger.info throughout the code to find the line with the error.

My suggestion is to add more preconditions before submitting the request, such as for audit log reason and sending messages, to make it easier for developers to detect a bug if they have one. I don't know if this is possible, but regardless of preconditions I would like to see a mention of the function that sent the request in the stack trace, because I realize that even if you fill the whole library with a million preconditions you can easily miss something.

Below I attach an example of a typical stack trace, but imagine that there will be nothing after “an error 400”...

[2024-06-02 21:28:30.253] [DefaultDispatcher-worker-16] [ERROR] [c.k.k.e.c.a.slash.SlashCommand]: Error during execution of invite slash command (dev.kord.core.event.interaction.GuildChatInputCommandInteractionCreateEvent@52ab5d0a)
dev.kord.rest.request.KtorRequestException: REST request returned an error: 400   Invalid Form Body {"content":{"_errors":[{"code":"BASE_TYPE_MAX_LENGTH","message":"Must be 4000 or fewer in length."}]}}
    at dev.kord.rest.request.KtorRequestHandler.handle(KtorRequestHandler.kt:61)
    at dev.kord.rest.request.KtorRequestHandler$handle$1.invokeSuspend(KtorRequestHandler.kt)
    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
    at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
    at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:584)
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:811)
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:715)
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:702)
lukellmann commented 5 months ago

Try out stackTraceRecovery, it will give you nicer stacktraces (ktor has some internal coroutines machinery that destroys stacktraces). You'd enable it like this:

val kord = Kord(token) {
    stackTraceRecovery = true
}
DRSchlaubi commented 5 months ago

Arguably we should enable this by default, it has a performance impact of always generating a stack trace, however, that's an approach that JDA took (I think their equivalent of this is enabled by default)

lukellmann commented 5 months ago

Arguably we should enable this by default, it has a performance impact of always generating a stack trace, however, that's an approach that JDA took (I think their equivalent of this is enabled by default)

Sound like a good idea indeed. It can still be disabled if needed.