Skip to content

[Issue 7787][pulsar-client-cpp] Throw std::exception types #7798

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

Merged
merged 1 commit into from
Aug 24, 2020

Conversation

Bklyn
Copy link
Contributor

@Bklyn Bklyn commented Aug 11, 2020

  • Change throws of const char* to use std::exception types instead

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

  • Operations which formerly threw const char* now throw a type derived from std::exception

Documentation

The methods that can throw should be documented as such. This was not implemented.

@Bklyn Bklyn force-pushed the 7787-throw-std-exception branch from 7d737d8 to 1b36007 Compare August 11, 2020 17:03
@Bklyn
Copy link
Contributor Author

Bklyn commented Aug 11, 2020

Seems some tests are failing, but I didn't touch the Java code (CPP/Py tests are OK).

@Bklyn Bklyn force-pushed the 7787-throw-std-exception branch from a0c8d8a to 6edb98a Compare August 12, 2020 16:29
* Change throws of const char* to use std::exception types instead
@Bklyn Bklyn force-pushed the 7787-throw-std-exception branch from 6edb98a to 1d3713b Compare August 13, 2020 14:03
@BewareMyPower
Copy link
Contributor

/pulsarbot run-failure-checks

2 similar comments
@BewareMyPower
Copy link
Contributor

/pulsarbot run-failure-checks

@BewareMyPower
Copy link
Contributor

/pulsarbot run-failure-checks

@jiazhai jiazhai requested review from merlimat and murong00 August 21, 2020 14:17
@jiazhai jiazhai added this to the 2.7.0 milestone Aug 21, 2020
@Bklyn
Copy link
Contributor Author

Bklyn commented Aug 21, 2020

@jiazhai @BewareMyPower Are these sorts of transient test failures typical? Thanks for the assistance!

@BewareMyPower
Copy link
Contributor

@Bklyn Yeah, there're some flaky tests. Besides, I guess sometimes ci tests may fail because of the limited resource of docker containers.

@sijie sijie merged commit b74094e into apache:master Aug 24, 2020
lbenc135 pushed a commit to lbenc135/pulsar that referenced this pull request Sep 5, 2020
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.
lbenc135 pushed a commit to lbenc135/pulsar that referenced this pull request Sep 5, 2020
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.
lbenc135 pushed a commit to lbenc135/pulsar that referenced this pull request Sep 5, 2020
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.
wolfstudy pushed a commit that referenced this pull request Oct 30, 2020
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[pulsar-client-cpp] Should throw std::exception (or a subtype), not const char*
4 participants