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

Provide message for Exception classes #279

Closed
jeremy303 opened this issue Sep 8, 2017 · 12 comments
Closed

Provide message for Exception classes #279

jeremy303 opened this issue Sep 8, 2017 · 12 comments

Comments

@jeremy303
Copy link

Some (or all?) of RxAndroidBle's Exception classes currently return null for getMessage(), making it difficult to provide meaningful error messages in the event of unexpected exceptions.

@dariuszseweryn
Copy link
Owner

Hello @HolySamosa
Could you give more context on how/when would you expect to use the value returned by .getMessage()?

@dariuszseweryn
Copy link
Owner

@HolySamosa ping?

@dariuszseweryn
Copy link
Owner

Closing due to inactivity.

All exceptions are typed and can be checked in the sources what they mean. Showing contents of the Exception.getMessage() in the UI is a bad thing to do as most of the time it contains some description that is not understandable for the user. For the development purposes one can use .toString() which is implemented in all BleExceptions for quick debugging.

@jeremy303
Copy link
Author

@dariuszseweryn Sorry for disappearing on this. We were hit by Hurricane Irma just after this was filed, knocking out power and internet for the better part of a week. I missed the earlier notifications and I'm just now catching up...

I ran into this issue w/ code for handling unexpected errors in my application. While for the exception classes that most commonly occur the application provides a user-friendly message that doesn't make use of .getMessage(), in the event of unexpected errors there is an opportunity for the user to get more details about the error and in this case getMessage() is used. Also, this is a shared error handler at the presentation layer of my app, which knows nothing about RxAndroidBle.

Thanks for your patience and the great library. :-)

@dariuszseweryn
Copy link
Owner

Sorry to hear that.

We will evaluate the idea of adding messages. As a temporary solution you could map the RxAndroidBle exceptions to your own ones at the boundary of the layer that knows about the library. :)

@jeremy303
Copy link
Author

Mapping the exceptions is exactly what I am doing. :-)

And thanks for the kind words. I was fortunate in that we only were without power-- no damage to our home, just a mess of small tree limbs to clean up.

@dariuszseweryn
Copy link
Owner

If you are mapping the exceptions then why the need for the messages? As far as I remember the library only uses exceptions that are declared in /exceptions.

@dariuszseweryn
Copy link
Owner

Ping? :)

@jeremy303
Copy link
Author

I'm mapping the "reason" values of BleScanException, as that's the case where my application was seeing unexpected errors. I'm not mapping the others, as I haven't really ran into unexpected errors outside of scanning.

I completely agree with your assessment that it is not best practice to use getMessage() for generating common user-facing messages to the user, however there are cases where a developer may need to make use of them, such as my case of optionally provided additional detail of the cause of an unexpected error to a technical end user.

I don't think this is pressing, but it would be welcome update. Thanks!

@dariuszseweryn
Copy link
Owner

I am thinking if this solution would be feasible?

@Override
public String getMessage() {
    return toString();
}

All library errors print with as much details as possible.

@uKL
Copy link
Collaborator

uKL commented Nov 2, 2017

Also better messages will be provided by #303

@uKL
Copy link
Collaborator

uKL commented Nov 2, 2017

Fixed by #303, closing.

@uKL uKL closed this as completed Nov 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants