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

ServerException vs KeenQueryClientException #101

Open
masojus opened this issue May 17, 2017 · 1 comment
Open

ServerException vs KeenQueryClientException #101

masojus opened this issue May 17, 2017 · 1 comment

Comments

@masojus
Copy link
Collaborator

masojus commented May 17, 2017

KeenQueryClientException seems to have been created as the exception that should be thrown out of KeenQueryClient and the comment says that even includes errors reported by the server:

Exceptions thrown by KeenQueryClient. This includes errors reported by the server.

But, there are several places where we throw ServerException out of KeenQueryClient instead. Is there a reason for this, or was it just an oversight? What should we throw in those cases?

What about for new code like Saved/Cached Query code in PR #96? That's not technically KeenQueryClient so is a new exception class desirable?

@masojus
Copy link
Collaborator Author

masojus commented May 17, 2017

Also, changing the exception type thrown out of a core method could alter how client code works since they'll no longer catch the right exception, but I'm not sure how changing an exceptional case affects semver, so I wonder if that type of change would have to wait for a major revision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant