Update the KafkaException using modern practices

[downfa11]

KafkaException(const char* filename, std::size_t lineno, const Error& error)
        : _when(std::chrono::system_clock::now()),
          _filename(filename),
          _lineno(lineno),
          _error(std::make_shared<Error>(error))
    {}

In the existing method, filename and lineno are expressed as follows, but in C++20, you can use the source_location feature.

KafkaException(const std::source_location location = std::source_location::current(), const Error& error)
        : _when(std::chrono::system_clock::now()),
          _filename(location.file_name()),
          _lineno(location.line()),
          _error(std::make_shared<Error>(error))
    {}

It may not be very significant, but I believe it is meaningful as it aligns with modern C++ syntax for this project.

@alex-q-chen sorry. I'm still learning and made some mistakes

[downfa11]

@mimiflynn could you please review this?

[kenneth-jia]

• the std::source_location is quit an elegant choice, but it's not supported until c++20. (the project has to be compatible with c++14/17)
• default parameter values must be specified starting from the rightmost parameter and moving leftwards
• if we're using std::source_location as the default parameter, we could get rid of the macro KAFKA THROW_ERROR as well -- just throw KafkaException(error) at where we need it.

[downfa11]

Thanks for the feedback!

As far as I know, source_location was originally provided in Boost before being adopted into C++20.

Since project already depends on Boost (e.g., for boost::optional), using boost::source_location will not introduce any new external dependencies.

I also plan to update the code so that exceptions can be thrown directly via KafkaException(error) and remove the KAFKA_THROW_ERROR macro in the process.

What do you think?

[kenneth-jia]

Yes, we have to use some trick for the optional, -- https://github.com/morganstanley/modern-cpp-kafka/blob/main/include/kafka/Types.h#L19
It also make the dependencies messy, https://github.com/morganstanley/modern-cpp-kafka/blob/main/.github/workflows/kafka_api_ci_tests.yml#L307

[downfa11]

While the boost::source_location solution is certainly elegant, I'm wondering if introducing a new Boost dependency might be a concern for the project.

I'd like to make sure we aren't adding complexity where we're trying to remove it.