-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat(REST): append additional information to the required User Agent #6112
Conversation
Why not making it a string instead of an array? |
It takes the approach of RFC7231. Each segment is divided, and it would be useful if those are reflected in the client option as well. |
I think the validateOptions method should be updated as well discord.js/src/client/Client.js Line 468 in 72473e0
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional destructuring, but otherwise LGTM
Co-authored-by: Vlad Frangu <kingdgrizzle@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a fan of this code, since it's the checks and user agent are done in every request rather than doing it only once (maybe in the RESTManager?).
I won't block, we can seek an alternative with a smaller performance impact later in another PR.
@kyranet - I did not notice that the APIRequest was created on every request. Of course, now it's quite obvious and it was my mistake for not looking into it more. We can look into quite possibly creating the final user agent at startup of the bot, rather than on every request. |
Please describe the changes this PR makes and why it should be merged:
This PR allows you to append information about the bot (like a website, version) to the end of the (already required) user agent. As specified in the API documentations, users should be able to do so and are encouraged to do it.
Discord.js users could alternatively modify the constants file, where the user agent string is defined, but it can lead to them not following the API specifications.
After having a talk with a few people, I decided to implement a
userAgentSuffix
option toClientOptions
for people to do this. Types are already updated. An example of the usage is down below.The implementation of this PR would append the array, joined by
,
, to the end of the already-default user agent.DiscordBot ($djsurl, $djsversion) Node.js/$nodeversion
DiscordBot ($djsurl, $djsversion) Node.js/$nodeversion, MyBot/$botversion
Status and versioning classification: