BewareMyPower commented on a change in pull request #14587:
URL: https://github.com/apache/pulsar/pull/14587#discussion_r821280374
##########
File path: pulsar-client-cpp/lib/ClientConnection.cc
##########
@@ -173,12 +172,20 @@ ClientConnection::ClientConnection(const std::string&
logicalAddress, const std:
physicalAddress_(physicalAddress),
cnxString_("[<none> -> " + physicalAddress + "] "),
incomingBuffer_(SharedBuffer::allocate(DefaultBufferSize)),
-
connectTimeoutTask_(std::make_shared<PeriodicTask>(executor_->getIOService(),
-
clientConfiguration.getConnectionTimeout())),
outgoingBuffer_(SharedBuffer::allocate(DefaultBufferSize)),
- consumerStatsRequestTimer_(executor_->createDeadlineTimer()),
maxPendingLookupRequest_(clientConfiguration.getConcurrentLookupRequest()) {
+ try {
+ socket_ = executor_->createSocket();
+ connectTimeoutTask_ =
std::make_shared<PeriodicTask>(executor_->getIOService(),
+
clientConfiguration.getConnectionTimeout());
+ consumerStatsRequestTimer_ = executor_->createDeadlineTimer();
+ } catch (std::exception& e) {
Review comment:
It's better to catch a `boost::system::system_error` here instead of a
raw `std::exception`.
BTW, I think the constructor should not throw an exception, see
https://github.com/apache/pulsar/blob/32c3cd1009eea884e0701143fe01dadf4556e73b/pulsar-client-cpp/lib/ExecutorService.cc#L63
It uses [the 1st overloaded
constructor](https://www.boost.org/doc/libs/1_77_0/doc/html/boost_asio/reference/basic_stream_socket/basic_stream_socket/overload1.html)
that doesn't open a socket, so no exception should be thrown.
Instead, the [3rd overloaded
constructor](https://www.boost.org/doc/libs/1_77_0/doc/html/boost_asio/reference/basic_stream_socket/basic_stream_socket/overload1.html)
opens the socket, in this case a `system_error` could be thrown.
>
[Exceptions](https://www.boost.org/doc/libs/1_77_0/doc/html/boost_asio/reference/basic_stream_socket/basic_stream_socket/overload3.html#boost_asio.reference.basic_stream_socket.basic_stream_socket.overload3.exceptions)
>
> boost::system::system_error
> Thrown on failure.
Could you confirm the exception was thrown here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
