Skip to content

Conversation

@pasmophobia
Copy link

This pull request updates how query parameters are handled when sending webhook requests in the Http client. The main change is that query parameters from the URL are now parsed and passed along with the request, rather than always being set to None.

Improvements to webhook request handling:

  • Query parameters from the URL are now parsed using url.query_pairs(), collected into a vector, and passed to the request if present, otherwise set to None (src/http/client.rs).
  • The params field in the Request struct is now set to the parsed query parameters instead of always being None (src/http/client.rs).

@github-actions github-actions bot added the http Related to the `http` module. label Aug 27, 2025
@mkrasnitski
Copy link
Collaborator

In serenity, the get_webhook_from_url function is equivalent to get_webhook_with_token in terms of functionality, and it makes requests to the same API endpoint. According to the Discord docs, the endpoints for getting a webhook object don't use any query params. What's the reason you want to do this?

@pasmophobia
Copy link
Author

The reason for handling query params here is that webhook URLs can include them (e.g. ?thread_id=).
Right now get_webhook_from_url drops that information, but the query string is semantically part of the webhook URL.
I think preserving it makes the behavior more intuitive.

@mkrasnitski
Copy link
Collaborator

The problem here is that the thread_id is not stored in the actual Webhook object returned from Discord when you ask for one, because Discord itself doesn't let you create a webhook directly in a thread. The user is instead expected to provide the query parameter each time a relevant API call is made, such as Execute Webhook or Get Webhook Message.

@Joshix-1
Copy link

Joshix-1 commented Sep 4, 2025

Handling the thread_id query param would make sense for the parse_webhook function in utils

@pasmophobia
Copy link
Author

Sorry, I misunderstood the code. I’ll open a new PR later.

@pasmophobia pasmophobia closed this Oct 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

http Related to the `http` module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants