Skip to content

Commit 165125c

Browse files
UgnineSirdisUgnineSirdis
committed
[Http] Reply with structured issues when client accepts json data
1 parent 514df1a commit 165125c

File tree

9 files changed

+99
-74
lines changed

9 files changed

+99
-74
lines changed

‎ydb/core/mon/async_http_mon.cpp‎

Lines changed: 44 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <ydb/core/grpc_services/base/base.h>
66
#include <ydb/core/base/ticket_parser.h>
77

8+
#include <library/cpp/json/json_writer.h>
89
#include <library/cpp/lwtrace/all.h>
910
#include <library/cpp/lwtrace/mon/mon_lwtrace.h>
1011
#include <ydb/library/actors/core/probes.h>
@@ -145,6 +146,11 @@ class THttpMonRequest : public NMonitoring::IMonHttpRequest {
145146
return {};
146147
}
147148

149+
bool AcceptsJsonResponse() {
150+
TStringBuf acceptHeader = GetHeader("Accept");
151+
return acceptHeader.find(TStringBuf("application/json")) != TStringBuf::npos;
152+
}
153+
148154
virtual TStringBuf GetCookie(TStringBuf name) const override {
149155
NHttp::TCookies cookies(GetHeader("Cookie"));
150156
return cookies.Get(name);
@@ -251,7 +257,7 @@ class THttpMonLegacyActorRequest : public TActorBootstrapped<THttpMonLegacyActor
251257
TString YdbToHttpError(Ydb::StatusIds::StatusCode status) {
252258
switch (status) {
253259
case Ydb::StatusIds::UNAUTHORIZED:
254-
return "401 Unauthorized";
260+
return "403 Forbidden";
255261
case Ydb::StatusIds::INTERNAL_ERROR:
256262
return "500 Internal Server Error";
257263
case Ydb::StatusIds::UNAVAILABLE:
@@ -268,26 +274,45 @@ class THttpMonLegacyActorRequest : public TActorBootstrapped<THttpMonLegacyActor
268274
}
269275

270276
void ReplyErrorAndPassAway(const NKikimr::NGRpcService::TEvRequestAuthAndCheckResult& result) {
277+
ReplyErrorAndPassAway(result.Status, result.Issues, true);
278+
}
279+
280+
void ReplyErrorAndPassAway(Ydb::StatusIds::StatusCode status, const NYql::TIssues& issues, bool addAccessControlHeaders) {
271281
NHttp::THttpIncomingRequestPtr request = Event->Get()->Request;
272-
NHttp::THeaders headers(request->Headers);
273282
TStringBuilder response;
274283
TStringBuilder body;
275-
const TString httpError = YdbToHttpError(result.Status);
276-
body << "<html><body><h1>" << httpError << "</h1>";
277-
if (result.Issues) {
278-
body << "<p>" << result.Issues.ToString() << "</p>";
279-
}
280-
body << "</body></html>";
281-
TString origin = TString(headers["Origin"]);
282-
if (origin.empty()) {
283-
origin = "*";
284+
TStringBuf contentType;
285+
const TString httpError = YdbToHttpError(status);
286+
287+
if (Container.AcceptsJsonResponse()) {
288+
contentType = "application/json";
289+
NJson::TJsonValue json;
290+
TString message;
291+
MakeJsonErrorReply(json, message, issues, NYdb::EStatus(status));
292+
NJson::WriteJson(&body.Out, &json);
293+
} else {
294+
contentType = "text/html";
295+
body << "<html><body><h1>" << httpError << "</h1>";
296+
if (issues) {
297+
body << "<p>" << issues.ToString() << "</p>";
298+
}
299+
body << "</body></html>";
284300
}
301+
285302
response << "HTTP/1.1 " << httpError << "\r\n";
286-
response << "Access-Control-Allow-Origin: " << origin << "\r\n";
287-
response << "Access-Control-Allow-Credentials: true\r\n";
288-
response << "Access-Control-Allow-Headers: Content-Type,Authorization,Origin,Accept\r\n";
289-
response << "Access-Control-Allow-Methods: OPTIONS, GET, POST, PUT, DELETE\r\n";
290-
response << "Content-Type: text/html\r\n";
303+
if (addAccessControlHeaders) {
304+
NHttp::THeaders headers(request->Headers);
305+
TString origin = TString(headers["Origin"]);
306+
if (origin.empty()) {
307+
origin = "*";
308+
}
309+
response << "Access-Control-Allow-Origin: " << origin << "\r\n";
310+
response << "Access-Control-Allow-Credentials: true\r\n";
311+
response << "Access-Control-Allow-Headers: Content-Type,Authorization,Origin,Accept\r\n";
312+
response << "Access-Control-Allow-Methods: OPTIONS, GET, POST, PUT, DELETE\r\n";
313+
}
314+
315+
response << "Content-Type: " << contentType << "\r\n";
291316
response << "Content-Length: " << body.Size() << "\r\n";
292317
response << "\r\n";
293318
response << body;
@@ -296,21 +321,9 @@ class THttpMonLegacyActorRequest : public TActorBootstrapped<THttpMonLegacyActor
296321
}
297322

298323
void ReplyForbiddenAndPassAway(const TString& error = {}) {
299-
NHttp::THttpIncomingRequestPtr request = Event->Get()->Request;
300-
TStringBuilder response;
301-
TStringBuilder body;
302-
body << "<html><body><h1>403 Forbidden</h1>";
303-
if (!error.empty()) {
304-
body << "<p>" << error << "</p>";
305-
}
306-
body << "</body></html>";
307-
response << "HTTP/1.1 403 Forbidden\r\n";
308-
response << "Content-Type: text/html\r\n";
309-
response << "Content-Length: " << body.Size() << "\r\n";
310-
response << "\r\n";
311-
response << body;
312-
ReplyWith(request->CreateResponseString(response));
313-
PassAway();
324+
NYql::TIssues issues;
325+
issues.AddIssue(error);
326+
ReplyErrorAndPassAway(Ydb::StatusIds::UNAUTHORIZED, issues, false);
314327
}
315328

316329
void SendRequest(const NKikimr::NGRpcService::TEvRequestAuthAndCheckResult* result = nullptr) {

‎ydb/core/mon/mon.cpp‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include <library/cpp/json/json_value.h>
1010
#include <library/cpp/json/json_reader.h>
11+
#include <library/cpp/protobuf/json/proto2json.h>
1112

1213
#include <util/string/ascii.h>
1314

@@ -88,6 +89,48 @@ NActors::IEventHandle* GetAuthorizeTicketResult(const NActors::TActorId& owner)
8889
}
8990
}
9091

92+
void MakeJsonErrorReply(NJson::TJsonValue& jsonResponse, TString& message, const NYdb::TStatus& status) {
93+
MakeJsonErrorReply(jsonResponse, message, status.GetIssues(), status.GetStatus());
94+
}
95+
96+
void MakeJsonErrorReply(NJson::TJsonValue& jsonResponse, TString& message, const NYql::TIssues& issues, NYdb::EStatus status) {
97+
google::protobuf::RepeatedPtrField<Ydb::Issue::IssueMessage> protoIssues;
98+
NYql::IssuesToMessage(issues, &protoIssues);
99+
100+
message.clear();
101+
102+
NJson::TJsonValue& jsonIssues = jsonResponse["issues"];
103+
for (const auto& queryIssue : protoIssues) {
104+
NJson::TJsonValue& issue = jsonIssues.AppendValue({});
105+
NProtobufJson::Proto2Json(queryIssue, issue);
106+
}
107+
108+
TString textStatus = TStringBuilder() << status;
109+
jsonResponse["status"] = textStatus;
110+
111+
// find first deepest error
112+
std::stable_sort(protoIssues.begin(), protoIssues.end(), [](const Ydb::Issue::IssueMessage& a, const Ydb::Issue::IssueMessage& b) -> bool {
113+
return a.severity() < b.severity();
114+
});
115+
116+
const google::protobuf::RepeatedPtrField<Ydb::Issue::IssueMessage>* protoIssuesPtr = &protoIssues;
117+
while (protoIssuesPtr->size() > 0 && protoIssuesPtr->at(0).issuesSize() > 0) {
118+
protoIssuesPtr = &protoIssuesPtr->at(0).issues();
119+
}
120+
121+
if (protoIssuesPtr->size() > 0) {
122+
const Ydb::Issue::IssueMessage& issue = protoIssuesPtr->at(0);
123+
NProtobufJson::Proto2Json(issue, jsonResponse["error"]);
124+
message = issue.message();
125+
} else {
126+
jsonResponse["error"]["message"] = textStatus;
127+
}
128+
129+
if (message.empty()) {
130+
message = textStatus;
131+
}
132+
}
133+
91134
IMonPage* TMon::RegisterActorPage(TIndexMonPage* index, const TString& relPath,
92135
const TString& title, bool preTag, TActorSystem* actorSystem, const TActorId& actorId, bool useAuth, bool sortPages) {
93136
return RegisterActorPage({

‎ydb/core/mon/mon.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#pragma once
22

3+
#include <library/cpp/json/writer/json_value.h>
34
#include <library/cpp/monlib/service/monservice.h>
45
#include <library/cpp/monlib/dynamic_counters/counters.h>
56
#include <library/cpp/monlib/service/pages/resources/css_mon_page.h>
@@ -10,12 +11,17 @@
1011

1112
#include <ydb/library/actors/core/actor.h>
1213
#include <ydb/library/actors/core/mon.h>
14+
#include <ydb/library/yql/public/issue/yql_issue.h>
15+
#include <ydb/public/sdk/cpp/client/ydb_types/status/status.h>
1316

1417
namespace NActors {
1518

1619
IEventHandle* SelectAuthorizationScheme(const NActors::TActorId& owner, NMonitoring::IMonHttpRequest& request);
1720
IEventHandle* GetAuthorizeTicketResult(const NActors::TActorId& owner);
1821

22+
void MakeJsonErrorReply(NJson::TJsonValue& jsonResponse, TString& message, const NYql::TIssues& issues, NYdb::EStatus status);
23+
void MakeJsonErrorReply(NJson::TJsonValue& jsonResponse, TString& message, const NYdb::TStatus& status);
24+
1925
class TActorSystem;
2026
struct TActorId;
2127

‎ydb/core/mon/ya.make‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,16 @@ SRCS(
1414
PEERDIR(
1515
library/cpp/json
1616
library/cpp/lwtrace/mon
17+
library/cpp/protobuf/json
1718
library/cpp/string_utils/url
1819
ydb/core/base
1920
ydb/core/grpc_services/base
2021
ydb/core/protos
2122
ydb/library/aclib
2223
ydb/library/actors/core
2324
ydb/library/actors/http
25+
ydb/library/yql/public/issue
26+
ydb/public/sdk/cpp/client/ydb_types/status
2427
)
2528

2629
END()

‎ydb/core/viewer/json_local_rpc.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ class TJsonLocalRpc : public TActorBootstrapped<TJsonLocalRpc<TProtoRequest, TPr
200200
if (!Result->Status->IsSuccess()) {
201201
NJson::TJsonValue json;
202202
TString message;
203-
MakeErrorReply(json, message, Result->Status.value());
203+
MakeJsonErrorReply(json, message, Result->Status.value());
204204
TStringStream stream;
205205
NJson::WriteJson(&stream, &json);
206206
if (Result->Status->GetStatus() == NYdb::EStatus::UNAUTHORIZED) {

‎ydb/core/viewer/viewer.cpp‎

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -733,44 +733,6 @@ TString TViewer::GetHTTPFORWARD(const TRequestState& request, const TString& loc
733733
return res;
734734
}
735735

736-
void MakeErrorReply(NJson::TJsonValue& jsonResponse, TString& message, const NYdb::TStatus& status) {
737-
google::protobuf::RepeatedPtrField<Ydb::Issue::IssueMessage> protoIssues;
738-
NYql::IssuesToMessage(status.GetIssues(), &protoIssues);
739-
740-
message.clear();
741-
742-
NJson::TJsonValue& jsonIssues = jsonResponse["issues"];
743-
for (const auto& queryIssue : protoIssues) {
744-
NJson::TJsonValue& issue = jsonIssues.AppendValue({});
745-
NProtobufJson::Proto2Json(queryIssue, issue);
746-
}
747-
748-
TString textStatus = TStringBuilder() << status.GetStatus();
749-
jsonResponse["status"] = textStatus;
750-
751-
// find first deepest error
752-
std::stable_sort(protoIssues.begin(), protoIssues.end(), [](const Ydb::Issue::IssueMessage& a, const Ydb::Issue::IssueMessage& b) -> bool {
753-
return a.severity() < b.severity();
754-
});
755-
756-
const google::protobuf::RepeatedPtrField<Ydb::Issue::IssueMessage>* protoIssuesPtr = &protoIssues;
757-
while (protoIssuesPtr->size() > 0 && protoIssuesPtr->at(0).issuesSize() > 0) {
758-
protoIssuesPtr = &protoIssuesPtr->at(0).issues();
759-
}
760-
761-
if (protoIssuesPtr->size() > 0) {
762-
const Ydb::Issue::IssueMessage& issue = protoIssuesPtr->at(0);
763-
NProtobufJson::Proto2Json(issue, jsonResponse["error"]);
764-
message = issue.message();
765-
} else {
766-
jsonResponse["error"]["message"] = textStatus;
767-
}
768-
769-
if (message.empty()) {
770-
message = textStatus;
771-
}
772-
}
773-
774736
NKikimrViewer::EFlag GetFlagFromTabletState(NKikimrWhiteboard::TTabletStateInfo::ETabletState state) {
775737
NKikimrViewer::EFlag flag = NKikimrViewer::EFlag::Grey;
776738
switch (state) {

‎ydb/core/viewer/viewer.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,8 +230,6 @@ void SplitIds(TStringBuf source, char delim, std::unordered_set<ValueType>& valu
230230
GenericSplitIds<ValueType>(source, delim, std::inserter(values, values.end()));
231231
}
232232

233-
void MakeErrorReply(NJson::TJsonValue& jsonResponse, TString& message, const NYdb::TStatus& status);
234-
235233
TString GetHTTPOKJSON();
236234
TString GetHTTPGATEWAYTIMEOUT();
237235
NKikimrViewer::EFlag GetFlagFromTabletState(NKikimrWhiteboard::TTabletStateInfo::ETabletState state);

‎ydb/core/viewer/viewer_query.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,7 @@ class TJsonQuery : public TViewerPipeClient {
506506
void MakeErrorReply(NJson::TJsonValue& jsonResponse, const NYdb::TStatus& status) {
507507
TString message;
508508

509-
NViewer::MakeErrorReply(jsonResponse, message, status);
509+
MakeJsonErrorReply(jsonResponse, message, status);
510510

511511
if (Span) {
512512
Span.EndError(message);
@@ -768,4 +768,3 @@ class TJsonQuery : public TViewerPipeClient {
768768
};
769769

770770
}
771-

‎ydb/core/viewer/ya.make‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,7 @@ PEERDIR(
551551
ydb/core/grpc_services
552552
ydb/core/grpc_services/local_rpc
553553
ydb/core/health_check
554+
ydb/core/mon
554555
ydb/core/node_whiteboard
555556
ydb/core/protos
556557
ydb/core/scheme

0 commit comments

Comments
 (0)