Added RdKafkaException base class

mfontanini / cppkafka

Modern C++ Apache Kafka client library (wrapper for librdkafka)

BSD 2-Clause "Simplified" License

589 stars 207 forks source link

Open accelerated opened 4 years ago

accelerated commented 4 years ago

Added RdKafkaException base class for exceptions which take an Error as parameter. This allows for easier catching and processing of kafka-specific errors.
In cases where the error message was sinked into the Exception (ParseException and ActionTerminatedException), the signature is pass-by-copy now to be consistent with the base Exception class. This should not break ABI since the constructor is only called internally.

accelerated commented 4 years ago

Hi @mfontanini, can you please take a look at this? ~thanks

mfontanini commented 4 years ago

While I'm okay with these changes and this should probably have been done this way from the start, this change breaks the ABI and API. This:

Assumes nobody is throwing these exceptions manually (e.g. while testing code).
Breaks the API as code calling e.g. QueueException::get_error will now fail to link.
(potentially) changes the layout of these exceptions. I think in practice they'll end up having the same layout but it's a risky change.

Having said that, I don't think it's worth bumping the major version for such a simple change.

accelerated commented 4 years ago

Agreed with you. I think it's a worthwhile change with small breakage risk.

accelerated commented 4 years ago

@mfontanini are you ok going forward with this? Can we merge it? Otherwise I'll close it.

accelerated commented 4 years ago

@mfontanini please check my last comment.