Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Coroutines Hiding Errors Within on_ready Event #1222

Closed
Ashthetik opened this issue Aug 12, 2024 · 11 comments
Closed

Coroutines Hiding Errors Within on_ready Event #1222

Ashthetik opened this issue Aug 12, 2024 · 11 comments
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@Ashthetik
Copy link
Contributor

Git Commit Reference

ced36fd

Describe the bug:

When utilising coroutines within the on_ready function, it's possible for D++ to ignore HTTP error codes and skip on printing any errors.
This specifically happens when a user creates a malformed Slash Command (see below) and proceeds to send it to Discord.

To Reproduce:

Utilising the basic layout of the example bot provided in the Using Slash Commands and Interactions wiki:

  1. Convert the on_ready to a coroutine utilisng -> dpp::task<void>.
  2. Malform the command data either by leaving a field empty, or overpopulating (>100 characters) -- see Additional Context.
  3. Resume building the bot as usual.
  4. Observe as it fails to output the error.

Expected Behaviour:

As expected with a synchronous command creation, it would also be expected that any error codes would also mirror through with a coroutine setup.

Screenshots:

DPP_Bug_Report_Ev1

System Details:

  • OS: Linux fedora 6.10.3-200.fc40.x86_64
  • Version: 40

Additional Context:

Example Minimal Code:

#include <dpp/dpp.h>

int main() {
    dpp::cluster bot("...");

    bot.on_ready([&bot, logs](const dpp::ready_t &) -> dpp::task<void> {
        if (dpp::run_once<struct bulk_register>()) {
            const dpp::slashcommand bancmd(
                "", // Empty Field
                // Overpopulated Field
               "...............................................................................................................................................................................................................................................   ", 
                bot.me.id
            );

            co_await bot.co_global_command_create(bancmd);
        }

        co_return;
    });

    bot.start(dpp::st_wait);

    return 0;
}   
@Ashthetik Ashthetik added the bug Something isn't working label Aug 12, 2024
@Mishura4
Copy link
Member

Not a bug - you need to print the error yourself with coroutines
The non-coroutine version passes a default callback that prints errors, this wouldn't be feasible with coroutines

With that said we could document this better

@Ashthetik
Copy link
Contributor Author

Noted.
Treating it respectively with confirmation_callback_t and is_error() mimics it enough.

Never knew coroutines would break the logic flow I had. The more you know 👍

@Ashthetik
Copy link
Contributor Author

Is it all good to close this, or should I keep it open for tracking purposes for a docs update? I'd love to PR it, but I suck at formal documentation, lol

@Jaskowicz1
Copy link
Contributor

Is it all good to close this, or should I keep it open for tracking purposes for a docs update? I'd love to PR it, but I suck at formal documentation, lol

Feel free to PR it! We can always say if something needs to be described more. Besides, some pages in our docs aren't too formal anyways (look at this page for example).

@Jaskowicz1 Jaskowicz1 added documentation Improvements or additions to documentation and removed bug Something isn't working labels Aug 25, 2024
@Jaskowicz1
Copy link
Contributor

Updated the labels to reflect @Mishura4 's comment as this isn't a bug but rather a lack of documentation.

@Jaskowicz1
Copy link
Contributor

Any news on this?

@braindigitalis
Copy link
Contributor

is there actually anything to do here? not sure anything needs to be added?

@Mishura4
Copy link
Member

Documentation

@Ashthetik
Copy link
Contributor Author

Yeah I was supposed to make a PR for the docs on this but forgot

Ashthetik added a commit to Ashthetik/DPP that referenced this issue Oct 17, 2024
Extends the current coroutine intro to include a generalised warning regarding error fall throughs
@Ashthetik
Copy link
Contributor Author

All good to close this now with the doc update being merged? Or are we keeping it up for visibility?

@Jaskowicz1
Copy link
Contributor

Closed as of #1288

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

4 participants