YT-23616: Remove HttpCode from TErrorResponse

hiddenpath · hiddenpath · commit b8ac60a673bd · 2025-02-18T13:18:25.000+03:00
commit_hash:412a7a1e02eb68d388aff73a439e98f6f2dab8a6
diff --git a/yt/cpp/mapreduce/client/transaction_pinger.cpp b/yt/cpp/mapreduce/client/transaction_pinger.cpp
@@ -40,25 +40,23 @@ namespace {
 
 void CheckError(const TString& requestId, NHttp::IResponsePtr response)
 {
-    TErrorResponse errorResponse(static_cast<int>(response->GetStatusCode()), requestId);
-
     if (const auto* ytError = response->GetHeaders()->Find("X-YT-Error")) {
-        errorResponse.ParseFromJsonError(*ytError);
-    }
-    if (errorResponse.IsOk()) {
-        return;
-    }
+        TYtError error;
+        error.ParseFrom(*ytError);
 
-    YT_LOG_ERROR("RSP %v - HTTP %v - %v",
+        TErrorResponse errorResponse(std::move(error), requestId);
+        if (errorResponse.IsOk()) {
+            return;
+        }
+
+        YT_LOG_ERROR("RSP %v - HTTP %v - %v",
             requestId,
             response->GetStatusCode(),
             errorResponse.AsStrBuf());
 
-    ythrow errorResponse;
-
-////////////////////////////////////////////////////////////////////////////////
-
-} // namespace
+        ythrow errorResponse;
+    }
+}
 
 void PingTx(NHttp::IClientPtr httpClient, const TPingableTransaction& tx)
 {
diff --git a/yt/cpp/mapreduce/http/http.cpp b/yt/cpp/mapreduce/http/http.cpp
@@ -766,14 +766,12 @@ THttpResponse::THttpResponse(
         return;
     }
 
-    ErrorResponse_ = TErrorResponse(HttpCode_, Context_.RequestId);
-
     auto logAndSetError = [&] (int code, const TString& rawError) {
         YT_LOG_ERROR("RSP %v - HTTP %v - %v",
             Context_.RequestId,
             HttpCode_,
             rawError.data());
-        ErrorResponse_->SetError(TYtError(code, rawError));
+        ErrorResponse_ = TErrorResponse(TYtError(code, rawError), Context_.RequestId);
     };
 
     switch (HttpCode_) {
@@ -807,8 +805,7 @@ THttpResponse::THttpResponse(
                     ExtendGenericError(*ErrorResponse_, NClusterErrorCodes::NBus::TransportError, "transport error");
                 }
             } else {
-                ErrorResponse_->SetRawError(
-                    errorString + " - X-YT-Error is missing in headers");
+                ErrorResponse_ = TErrorResponse(TYtError(errorString + " - X-YT-Error is missing in headers"), Context_.RequestId);
             }
             break;
         }
@@ -854,8 +851,9 @@ TMaybe<TErrorResponse> THttpResponse::ParseError(const THttpHeaders& headers)
 {
     for (const auto& header : headers) {
         if (header.Name() == "X-YT-Error") {
-            TErrorResponse errorResponse(HttpCode_, Context_.RequestId);
-            errorResponse.ParseFromJsonError(header.Value());
+            TYtError error;
+            error.ParseFrom(header.Value());
+            TErrorResponse errorResponse(std::move(error), Context_.RequestId);
             if (errorResponse.IsOk()) {
                 return Nothing();
             }
diff --git a/yt/cpp/mapreduce/http/http_client.cpp b/yt/cpp/mapreduce/http/http_client.cpp
@@ -41,25 +41,21 @@ TMaybe<TErrorResponse> GetErrorResponse(const TString& hostName, const TString&
         return {};
     }
 
-    TErrorResponse errorResponse(static_cast<int>(httpCode), requestId);
-
     auto logAndSetError = [&] (int code, const TString& rawError) {
         YT_LOG_ERROR("RSP %v - HTTP %v - %v",
             requestId,
             httpCode,
             rawError.data());
-        errorResponse.SetError(TYtError(code, rawError));
+        return TErrorResponse(TYtError(code, rawError), requestId);
     };
 
 
     switch (httpCode) {
         case NHttp::EStatusCode::TooManyRequests:
-            logAndSetError(NClusterErrorCodes::NSecurityClient::RequestQueueSizeLimitExceeded, "request rate limit exceeded");
-            break;
+            return logAndSetError(NClusterErrorCodes::NSecurityClient::RequestQueueSizeLimitExceeded, "request rate limit exceeded");
 
         case NHttp::EStatusCode::InternalServerError:
-            logAndSetError(NClusterErrorCodes::NRpc::Unavailable, "internal error in proxy " + hostName);
-            break;
+            return logAndSetError(NClusterErrorCodes::NRpc::Unavailable, "internal error in proxy " + hostName);
 
         default: {
             TStringStream httpHeaders;
@@ -78,7 +74,10 @@ TMaybe<TErrorResponse> GetErrorResponse(const TString& hostName, const TString&
                 errorString.data());
 
             if (auto errorHeader = response->GetHeaders()->Find("X-YT-Error")) {
-                errorResponse.ParseFromJsonError(*errorHeader);
+                TYtError error;
+                error.ParseFrom(*errorHeader);
+
+                TErrorResponse errorResponse(std::move(error), requestId);
                 if (errorResponse.IsOk()) {
                     return Nothing();
                 }
@@ -88,13 +87,9 @@ TMaybe<TErrorResponse> GetErrorResponse(const TString& hostName, const TString&
                 return errorResponse;
             }
 
-            errorResponse.SetRawError(
-                    errorString + " - X-YT-Error is missing in headers");
-            break;
+            return TErrorResponse(TYtError(errorString + " - X-YT-Error is missing in headers"), requestId);
         }
     }
-
-    return errorResponse;
 }
 
 void CheckErrorResponse(const TString& hostName, const TString& requestId, const NHttp::IResponsePtr& response)
@@ -317,8 +312,9 @@ class TCoreHttpResponse
         TMaybe<TErrorResponse> ParseError(const NHttp::THeadersPtr& headers)
         {
             if (auto errorHeader = headers->Find("X-YT-Error")) {
-                TErrorResponse errorResponse(static_cast<int>(Response_->GetStatusCode()), RequestId_);
-                errorResponse.ParseFromJsonError(*errorHeader);
+                TYtError error;
+                error.ParseFrom(*errorHeader);
+                TErrorResponse errorResponse(std::move(error), RequestId_);
                 if (errorResponse.IsOk()) {
                     return Nothing();
                 }
diff --git a/yt/cpp/mapreduce/http_client/raw_batch_request.cpp b/yt/cpp/mapreduce/http_client/raw_batch_request.cpp
@@ -695,8 +695,7 @@ void THttpRawBatchRequest::ParseResponse(
                 if (errorIt == responseNode.end()) {
                     BatchItemList_[i].ResponseParser->SetResponse(Nothing());
                 } else {
-                    TErrorResponse error(400, requestId);
-                    error.SetError(TYtError(errorIt->second));
+                    TErrorResponse error(TYtError(errorIt->second), requestId);
                     if (auto curInterval = IsRetriable(error) ? retryPolicy->OnRetriableError(error) : Nothing()) {
                         YT_LOG_INFO(
                             "Batch subrequest (%s) failed, will retry, error: %s",
diff --git a/yt/cpp/mapreduce/interface/errors.cpp b/yt/cpp/mapreduce/interface/errors.cpp
@@ -280,11 +280,6 @@ TString TYtError::FullDescription() const
 
 ////////////////////////////////////////////////////////////////////////////////
 
-TErrorResponse::TErrorResponse(int httpCode, const TString& requestId)
-    : HttpCode_(httpCode)
-    , RequestId_(requestId)
-{ }
-
 TErrorResponse::TErrorResponse(TYtError error, const TString& requestId)
     : RequestId_(requestId)
     , Error_(std::move(error))
@@ -320,11 +315,6 @@ void TErrorResponse::SetIsFromTrailers(bool isFromTrailers)
     IsFromTrailers_ = isFromTrailers;
 }
 
-int TErrorResponse::GetHttpCode() const
-{
-    return HttpCode_;
-}
-
 bool TErrorResponse::IsFromTrailers() const
 {
     return IsFromTrailers_;
diff --git a/yt/cpp/mapreduce/interface/errors.h b/yt/cpp/mapreduce/interface/errors.h
@@ -157,8 +157,6 @@ class TErrorResponse
     : public yexception
 {
 public:
-    TErrorResponse(int httpCode, const TString& requestId);
-
     TErrorResponse(TYtError error, const TString& requestId);
 
     /// Get error object returned by server.
@@ -167,9 +165,6 @@ class TErrorResponse
     /// Get if (correlation-id) of request that was responded with error.
     TString GetRequestId() const;
 
-    /// Get HTTP code of response.
-    int GetHttpCode() const;
-
     /// Is error parsed from response trailers.
     bool IsFromTrailers() const;
 
@@ -218,7 +213,6 @@ class TErrorResponse
     void Setup();
 
 private:
-    int HttpCode_;
     TString RequestId_;
     TYtError Error_;
     bool IsFromTrailers_ = false;

Original file line number	Diff line number	Diff line change
`@@ -766,14 +766,12 @@ THttpResponse::THttpResponse(`
`766`	`766`	`return;`
`767`	`767`	`}`
`768`	`768`
`769`		`- ErrorResponse_ = TErrorResponse(HttpCode_, Context_.RequestId);`
`770`		`-`
`771`	`769`	`auto logAndSetError = [&] (int code, const TString& rawError) {`
`772`	`770`	`YT_LOG_ERROR("RSP %v - HTTP %v - %v",`
`773`	`771`	`Context_.RequestId,`
`774`	`772`	`HttpCode_,`
`775`	`773`	`rawError.data());`
`776`		`- ErrorResponse_->SetError(TYtError(code, rawError));`
	`774`	`+ ErrorResponse_ = TErrorResponse(TYtError(code, rawError), Context_.RequestId);`
`777`	`775`	`};`
`778`	`776`
`779`	`777`	`switch (HttpCode_) {`
`@@ -807,8 +805,7 @@ THttpResponse::THttpResponse(`
`807`	`805`	`ExtendGenericError(*ErrorResponse_, NClusterErrorCodes::NBus::TransportError, "transport error");`
`808`	`806`	`}`
`809`	`807`	`} else {`
`810`		`- ErrorResponse_->SetRawError(`
`811`		`- errorString + " - X-YT-Error is missing in headers");`
	`808`	`+ ErrorResponse_ = TErrorResponse(TYtError(errorString + " - X-YT-Error is missing in headers"), Context_.RequestId);`
`812`	`809`	`}`
`813`	`810`	`break;`
`814`	`811`	`}`
`@@ -854,8 +851,9 @@ TMaybe<TErrorResponse> THttpResponse::ParseError(const THttpHeaders& headers)`
`854`	`851`	`{`
`855`	`852`	`for (const auto& header : headers) {`
`856`	`853`	`if (header.Name() == "X-YT-Error") {`
`857`		`- TErrorResponse errorResponse(HttpCode_, Context_.RequestId);`
`858`		`- errorResponse.ParseFromJsonError(header.Value());`
	`854`	`+ TYtError error;`
	`855`	`+ error.ParseFrom(header.Value());`
	`856`	`+ TErrorResponse errorResponse(std::move(error), Context_.RequestId);`
`859`	`857`	`if (errorResponse.IsOk()) {`
`860`	`858`	`return Nothing();`
`861`	`859`	`}`