Copilot commented on code in PR #3791:
URL: https://github.com/apache/celeborn/pull/3791#discussion_r3756289316
##########
cpp/celeborn/network/MessageDispatcher.cpp:
##########
@@ -115,8 +120,10 @@ void MessageDispatcher::read(Context*,
std::unique_ptr<Message> toRecvMsg) {
failure->errorMsg());
LOG(ERROR) << errorMsg;
if (found) {
+ // Carry the streamChunkSlice context and the worker's error message so
+ // the reader's fetch-failure path sees the real cause.
holder.msgPromise.setException(
- folly::exception_wrapper(std::exception()));
+ folly::make_exception_wrapper<std::runtime_error>(errorMsg));
Review Comment:
CHUNK_FETCH_FAILURE now wraps a runtime_error that includes both
streamChunkSlice context and the worker error message, but
MessageDispatcherTest::sendFetchChunkRequestAndReceiveFailure still only
asserts hasException(). Adding an assertion on the exception message would
prevent regressions where the message is accidentally dropped again.
##########
cpp/celeborn/network/MessageDispatcher.cpp:
##########
@@ -63,8 +63,13 @@ void MessageDispatcher::read(Context*,
std::unique_ptr<Message> toRecvMsg) {
LOG(ERROR) << "Rpc failed, requestId: " << failure->requestId()
<< " errorMsg: " << failure->errorMsg() << std::endl;
if (found) {
+ // Carry the worker's error message on the exception so the push/fetch
+ // callbacks can recover the precise cause via
+ // ShuffleClientImpl::getPushDataFailCause. A blank std::exception
+ // would collapse every failure into the non-critical default.
holder.msgPromise.setException(
- folly::exception_wrapper(std::exception()));
+ folly::make_exception_wrapper<std::runtime_error>(
+ failure->errorMsg()));
Review Comment:
RpcFailure::errorMsg() returns a std::string by value, so calling it
multiple times here copies the message repeatedly. Since the message is used
for both logging and the exception, cache it once to avoid redundant copies
(and keep logging/exception consistent).
--
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]
