-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[Issue 7787][pulsar-client-cpp] Throw std::exception types #7798
Conversation
7d737d8
to
1b36007
Compare
Seems some tests are failing, but I didn't touch the Java code (CPP/Py tests are OK). |
a0c8d8a
to
6edb98a
Compare
* Change throws of const char* to use std::exception types instead
6edb98a
to
1d3713b
Compare
/pulsarbot run-failure-checks |
2 similar comments
/pulsarbot run-failure-checks |
/pulsarbot run-failure-checks |
@jiazhai @BewareMyPower Are these sorts of transient test failures typical? Thanks for the assistance! |
@Bklyn Yeah, there're some flaky tests. Besides, I guess sometimes ci tests may fail because of the limited resource of docker containers. |
Fixes apache#7787 ### Motivation Throw std::exception instead of `const char*` ### Modifications Should be self explanatory. ### Verifying this change This change should be covered by existing tests, such as BatchMessageTest. Not sure about the Auth test which catches `...` however.
Fixes apache#7787 ### Motivation Throw std::exception instead of `const char*` ### Modifications Should be self explanatory. ### Verifying this change This change should be covered by existing tests, such as BatchMessageTest. Not sure about the Auth test which catches `...` however.
Fixes apache#7787 ### Motivation Throw std::exception instead of `const char*` ### Modifications Should be self explanatory. ### Verifying this change This change should be covered by existing tests, such as BatchMessageTest. Not sure about the Auth test which catches `...` however.
Fixes #7787 ### Motivation Throw std::exception instead of `const char*` ### Modifications Should be self explanatory. ### Verifying this change This change should be covered by existing tests, such as BatchMessageTest. Not sure about the Auth test which catches `...` however. (cherry picked from commit b74094e)
Fixes #7787
Motivation
Throw std::exception instead of
const char*
Modifications
Should be self explanatory.
Verifying this change
This change should be covered by existing tests, such as BatchMessageTest. Not sure about the Auth test which catches
...
however.Does this pull request potentially affect one of the following parts:
Yes
const char*
now throw a type derived fromstd::exception
Documentation
The methods that can throw should be documented as such. This was not implemented.