Copilot commented on code in PR #3801:
URL: https://github.com/apache/celeborn/pull/3801#discussion_r3780731413
##########
cpp/celeborn/network/MessageDispatcher.cpp:
##########
@@ -17,10 +17,34 @@
#include "celeborn/network/MessageDispatcher.h"
+#include <folly/ExceptionWrapper.h>
+
#include "celeborn/protocol/TransportMessage.h"
namespace celeborn {
namespace network {
+namespace {
+// Builds a retriable "connection closed" error. Sending on a closed connection
+// is a normal recoverable condition -- the peer closed the socket because of a
+// worker restart or an idle timeout -- not an invariant violation. Reporting
it
+// through the promise instead of asserting mirrors the Java client, where a
+// send on an inactive channel surfaces as a retriable IOException via
+// TransportResponseHandler#channelInactive -> failOutstandingRequests, which
+// CelebornInputStream then retries or fails over to the replica.
+folly::exception_wrapper makeConnectionClosedException(
+ const std::string& detail) {
+ return folly::make_exception_wrapper<utils::CelebornRuntimeError>(
+ __FILE__,
+ static_cast<size_t>(__LINE__),
+ __FUNCTION__,
+ /*expression=*/"connection closed",
+ /*message=*/detail,
+ utils::error_source::kErrorSourceRuntime.c_str(),
+ utils::error_code::kInvalidState.c_str(),
+ /*isRetriable=*/true);
+}
Review Comment:
`makeConnectionClosedException()` hardcodes `__FUNCTION__` / `__LINE__` from
inside the helper, so all connection-closed failures will report the function
as `makeConnectionClosedException` and the same line number. This makes the
exception diagnostics less actionable (the call sites are `operator()`,
`sendFetchChunkRequest`, and `cleanup()`). Consider constructing the
`CelebornRuntimeError` at each call site (or passing
`__FILE__/__LINE__/__FUNCTION__` into the helper) so the exception points to
the actual failure location.
--
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]
