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

Include cause in ConfigurationException #143

Merged
merged 2 commits into from
Apr 20, 2017

Conversation

brianguertin
Copy link
Contributor

So, for debugging purposes, I would prefer to know the cause of this exception when there is one, and I don't see any harm in including it :)

@quinnjn
Copy link
Contributor

quinnjn commented Apr 11, 2017

Hi @brianguertin! Thanks for the PR!

I'm intrigued that you had to debug something in BraintreeFragment. Do you mind explaining what you were debugging? Also, how would knowing the stack trace have made this debugging easier?

@brianguertin
Copy link
Contributor Author

brianguertin commented Apr 18, 2017

Oh, just noticed your comment.

The reason I want to differentiate is because I prefer handle network connectivity issues in a different manner (E.g. informing the user they are offline or have no internet connectivity). I also ensure that general connectivity issues don't raise alerts in our app's bug tracking tools (because lack of internet is not a bug that needs fixing).

Currently, ConfigurationException swallows connectivity issues, so I can't detect them and handle them differently.

So in production, most ConfigurationException are just noise. If I suppress all of them, then I won't be alerted to legitimate configuration errors.

@@ -4,6 +4,9 @@
* Error class thrown when a configuration value is invalid
*/
public class ConfigurationException extends BraintreeException {
public ConfigurationException(String message, Throwable cause) {
Copy link
Member

Choose a reason for hiding this comment

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

Needs a new line before this line.

@@ -4,6 +4,9 @@
* Error class thrown when a configuration value is invalid
*/
public class ConfigurationException extends BraintreeException {
public ConfigurationException(String message, Throwable cause) {
super(message, cause);
}
Copy link
Member

Choose a reason for hiding this comment

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

Needs a new line after this line.

@lkorth
Copy link
Member

lkorth commented Apr 20, 2017

Thanks again for the PR @brianguertin! Just two whitespace changes and we'll get this merged in.

@lkorth lkorth merged commit b1ef3c5 into braintree:master Apr 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants