Skip to content

Conversation

@Feinq
Copy link
Contributor

@Feinq Feinq commented Jul 30, 2025

Summary

This is a refactoring and stability improvement for the coroutine helper functions on message_create_t and interaction_create_t.

Following a review discussion, all relevant co_ helpers have been updated to use the modern "pass-by-value and move" C++ idiom. This change achieves two primary goals:

  1. It fixes a critical dangling reference bug present in the original implementations that could lead to use-after-free errors and crashes.
  2. It simplifies the API by replacing redundant const& and && overloads with a single, more efficient signature.

To support this refactoring, interaction_modal_response was also made fully copyable and movable by implementing the Rule of 5.

While the Rule of 0 would be preferred, this was not possible due to the class's explicitly declared virtual destructor.

This PR also introduces the follow_up function as originally intended and includes several other minor improvements discovered during implementation.

Changes

  • Refactoring & Bug Fix:
    • Refactored all co_ helpers (co_reply, co_send, co_edit_response, etc.) to take parameters by value. This corrects a systemic dangling reference bug and modernizes the API.
    • Explicitly defaulted the Rule of 5 (copy/move constructors and assignment operators) for interaction_modal_response to make it a properly movable type, enabling its use with the new pass-by-value pattern.
  • New Feature:
    • Added follow_up and co_follow_up functions to interaction_create_t.
  • Other Improvements:
    • API Consistency: Added a co_edit_original_response overload that accepts a std::string for better consistency with similar functions.
    • Performance: The for-loop in edit_original_response now uses const& to avoid unnecessary copies of file data.
  • Documentation: Corrected several doxygen comments that incorrectly referred to "slash commands" instead of the more general "interactions."

Testing

All changes were thoroughly tested with a local bot, covering both lvalue (copy) and rvalue (move) call patterns for all refactored functions to ensure correctness and prevent regressions.

Code change checklist

  • I have ensured that all methods and functions are fully documented using doxygen style comments.
  • My code follows the coding style guide.
  • I tested that my change works before raising the PR.
  • I have ensured that I did not break any existing API calls.
  • I have not built my pull request using AI, a static analysis tool or similar without any human oversight.

@netlify
Copy link

netlify bot commented Jul 30, 2025

Deploy Preview for dpp-dev ready!

Name Link
🔨 Latest commit 6ac3abe
🔍 Latest deploy log https://app.netlify.com/projects/dpp-dev/deploys/688a59765c09060008172b10
😎 Deploy Preview https://deploy-preview-1467--dpp-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions github-actions bot added documentation Improvements or additions to documentation code Improvements or additions to code. labels Jul 30, 2025
The existing coroutine helper functions in message_create_t and interaction_create_t used an unsafe [&] capture, which could lead to dangling references. This commit corrects the issue by capturing all parameters by value or move.

Additionally, a new co_edit_original_response overload was added that accepts a const std::string& for consistency with other helper functions.
@Feinq Feinq changed the title feat: Added follow_up, co_follow_up functions and documentation improvements fix(async): Correct unsafe captures and add additional helpers Jul 30, 2025
Copy link
Member

@Mishura4 Mishura4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point the right thing to do with these co_ functions would be to pass by value and then move in the lambda capture. With that said that'll require a lot of changes to the PHP script so, I think we should accept this PR for now, but if you want to have a shot at the PHP script to do that, feel free to

Refactors all async helper functions on message_create_t and interaction_create_t to use the pass-by-value idiom. This fixes a potential critical dangling reference bug across the async API.

This change was enabled by making interaction_modal_response movable (Rule of Five).
@Feinq Feinq changed the title fix(async): Correct unsafe captures and add additional helpers refactor(async): Modernize coroutine helpers and add follow_up functions Jul 30, 2025
@Feinq
Copy link
Contributor Author

Feinq commented Jul 30, 2025

The PR description has been updated.

On a related note, I've noticed that this refactoring will make the LNK2019 error example in the troubleshooting documentation obsolete, as the mangled name for co_reply will change. See here.
Since I'm developing on Linux, I'm unable to generate the updated MSVC linker error to replace it.

Suggestions would be appreciated on this.

@braindigitalis braindigitalis merged commit e8a9d74 into brainboxdotcc:dev Jul 31, 2025
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code Improvements or additions to code. documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants