-
Notifications
You must be signed in to change notification settings - Fork 33
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
Add a better way to handle http exceptions #7
Add a better way to handle http exceptions #7
Conversation
Isn't the PR you did exactly the same as it was before? |
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.
Can you add some documentation for the added methods?
private final String body; | ||
|
||
public HttpException(int code, @NotNull String body) { | ||
super("Request returned failure " + code + ": " + body); |
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.
Maybe it makes sense to exclude the body from the message if its otherwise accessible
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.
Isn't the PR you did exactly the same as it was before?
Sending the messageRequest returned failure <code>: <body>
I excluded body & code from message in http exception, as @MinnDevelopment metioned. If user will log exception, message will remain the same as it was before. But if you catch it you're able to access exception body & code. For example now im able to catch 401 exceptions, when token is incorrect.
Can you tell me what to comment on? I'm quite new in open source and from my perspective there is not much to comment on here, only the constructor has changed and 2 getters have been added.
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 mean that you comment what the getters will return. Like this
discord-webhooks/src/main/java/club/minnced/discord/webhook/WebhookClient.java
Lines 133 to 142 in 530d603
/** | |
* The URL for this webhook formatted using {@link #WEBHOOK_URL} unless | |
* specified by {@link club.minnced.discord.webhook.WebhookClientBuilder#WebhookClientBuilder(String)} explicitly | |
* | |
* @return The URL for this webhook | |
*/ | |
@NotNull | |
public String getUrl() { | |
return url; | |
} |
I created documentation for new code, but i still think that docs for simple getters in exception are redundant. |
If something goes wrong while sending a message to the webhook, e.g. the token is incorrect. WebhookClient throws an exception with a message from which it is difficult to extract e.g. an error code. Pull request changes it so that error handling is simpler.