Skip to content

Commit 1064ddc

Browse files
adameatadameat
authored
change error format for http handlers (#6248)
1 parent a2023e5 commit 1064ddc

File tree

4 files changed

+72
-37
lines changed

4 files changed

+72
-37
lines changed

‎ydb/core/viewer/json_local_rpc.h

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#include <ydb/library/actors/core/actor_bootstrapped.h>
33
#include <ydb/library/actors/core/mon.h>
44
#include <library/cpp/protobuf/json/json2proto.h>
5+
#include <library/cpp/json/json_writer.h>
56
#include <ydb/core/base/tablet_pipe.h>
67
#include <ydb/library/services/services.pb.h>
78
#include <ydb/core/tx/schemeshard/schemeshard.h>
@@ -206,12 +207,15 @@ class TJsonLocalRpc : public TActorBootstrapped<TJsonLocalRpc<TProtoRequest, TPr
206207
void ReplyAndPassAway() {
207208
if (Result && Result->Status) {
208209
if (!Result->Status->IsSuccess()) {
210+
NJson::TJsonValue json;
211+
TString message;
212+
MakeErrorReply(json, message, Result->Status.value());
213+
TStringStream stream;
214+
NJson::WriteJson(&stream, &json);
209215
if (Result->Status->GetStatus() == NYdb::EStatus::UNAUTHORIZED) {
210-
return ReplyAndPassAway(Viewer->GetHTTPFORBIDDEN(Event->Get()));
216+
return ReplyAndPassAway(Viewer->GetHTTPFORBIDDEN(Event->Get(), "application/json", stream.Str()));
211217
} else {
212-
TStringStream error;
213-
Result->Status->Out(error);
214-
return ReplyAndPassAway(Viewer->GetHTTPBADREQUEST(Event->Get(), "text/plain", error.Str()));
218+
return ReplyAndPassAway(Viewer->GetHTTPBADREQUEST(Event->Get(), "application/json", stream.Str()));
215219
}
216220
} else {
217221
TStringStream json;

‎ydb/core/viewer/json_query.h

Lines changed: 16 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -398,13 +398,14 @@ class TJsonQuery : public TViewerPipeClient<TJsonQuery> {
398398
if (ev->Get()->Record.GetRef().GetYdbStatus() == Ydb::StatusIds::SUCCESS) {
399399
QueryResponse.Set(std::move(ev));
400400
MakeOkReply(jsonResponse, QueryResponse->Record.GetRef());
401+
if (Schema == ESchemaType::Classic && Stats.empty() && (Action.empty() || Action == "execute")) {
402+
jsonResponse = std::move(jsonResponse["result"]);
403+
}
401404
} else {
402405
QueryResponse.Error("QueryError");
403-
MakeErrorReply(jsonResponse, ev->Get()->Record.GetRef().MutableResponse()->MutableQueryIssues());
404-
}
405-
406-
if (Schema == ESchemaType::Classic && Stats.empty() && (Action.empty() || Action == "execute")) {
407-
jsonResponse = std::move(jsonResponse["result"]);
406+
NYql::TIssues issues;
407+
NYql::IssuesFromMessage(ev->Get()->Record.GetRef().GetResponse().GetQueryIssues(), issues);
408+
MakeErrorReply(jsonResponse, NYdb::TStatus(NYdb::EStatus(ev->Get()->Record.GetRef().GetYdbStatus()), std::move(issues)));
408409
}
409410

410411
TStringStream stream;
@@ -421,7 +422,9 @@ class TJsonQuery : public TViewerPipeClient<TJsonQuery> {
421422
auto& record(ev->Get()->Record);
422423
NJson::TJsonValue jsonResponse;
423424
if (record.IssuesSize() > 0) {
424-
MakeErrorReply(jsonResponse, record.MutableIssues());
425+
NYql::TIssues issues;
426+
NYql::IssuesFromMessage(record.GetIssues(), issues);
427+
MakeErrorReply(jsonResponse, NYdb::TStatus(NYdb::EStatus(record.GetStatusCode()), std::move(issues)));
425428
}
426429

427430
TStringStream stream;
@@ -472,26 +475,11 @@ class TJsonQuery : public TViewerPipeClient<TJsonQuery> {
472475
}
473476

474477
private:
475-
void MakeErrorReply(NJson::TJsonValue& jsonResponse, google::protobuf::RepeatedPtrField<Ydb::Issue::IssueMessage>* protoIssues) {
476-
NJson::TJsonValue& jsonIssues = jsonResponse["issues"];
477-
478-
// find first deepest error
479-
std::stable_sort(protoIssues->begin(), protoIssues->end(), [](const Ydb::Issue::IssueMessage& a, const Ydb::Issue::IssueMessage& b) -> bool {
480-
return a.severity() < b.severity();
481-
});
482-
while (protoIssues->size() > 0 && (*protoIssues)[0].issuesSize() > 0) {
483-
protoIssues = (*protoIssues)[0].mutable_issues();
484-
}
478+
void MakeErrorReply(NJson::TJsonValue& jsonResponse, const NYdb::TStatus& status) {
485479
TString message;
486-
if (protoIssues->size() > 0) {
487-
const Ydb::Issue::IssueMessage& issue = (*protoIssues)[0];
488-
NProtobufJson::Proto2Json(issue, jsonResponse["error"]);
489-
message = issue.message();
490-
}
491-
for (const auto& queryIssue : *protoIssues) {
492-
NJson::TJsonValue& issue = jsonIssues.AppendValue({});
493-
NProtobufJson::Proto2Json(queryIssue, issue);
494-
}
480+
481+
NViewer::MakeErrorReply(jsonResponse, message, status);
482+
495483
if (Span) {
496484
Span.EndError("Error");
497485
}
@@ -513,11 +501,9 @@ class TJsonQuery : public TViewerPipeClient<TJsonQuery> {
513501
}
514502
}
515503
catch (const std::exception& ex) {
516-
google::protobuf::RepeatedPtrField<Ydb::Issue::IssueMessage> protoIssues;
517-
Ydb::Issue::IssueMessage* issue = protoIssues.Add();
518-
issue->set_message(TStringBuilder() << "Convert error: " << ex.what());
519-
issue->set_severity(NYql::TSeverityIds::S_ERROR);
520-
MakeErrorReply(jsonResponse, &protoIssues);
504+
NYql::TIssues issues;
505+
issues.AddIssue(TStringBuilder() << "Convert error: " << ex.what());
506+
MakeErrorReply(jsonResponse, NYdb::TStatus(NYdb::EStatus::BAD_REQUEST, std::move(issues)));
521507
return;
522508
}
523509
}

‎ydb/core/viewer/viewer.cpp

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include <library/cpp/lwtrace/mon/mon_lwtrace.h>
1616
#include <contrib/libs/yaml-cpp/include/yaml-cpp/yaml.h>
1717
#include <library/cpp/yaml/as/tstring.h>
18+
#include <library/cpp/protobuf/json/proto2json.h>
1819
#include <util/system/fstat.h>
1920
#include <util/stream/file.h>
2021
#include "viewer.h"
@@ -211,7 +212,7 @@ class TViewer : public TActorBootstrapped<TViewer>, public IViewer {
211212
TString GetHTTPOK(const TRequestState& request, TString type, TString response, TInstant lastModified) override;
212213
TString GetHTTPGATEWAYTIMEOUT(const TRequestState& request, TString type, TString response) override;
213214
TString GetHTTPBADREQUEST(const TRequestState& request, TString type, TString response) override;
214-
TString GetHTTPFORBIDDEN(const TRequestState& request) override;
215+
TString GetHTTPFORBIDDEN(const TRequestState& request, TString type, TString response) override;
215216
TString GetHTTPNOTFOUND(const TRequestState& request) override;
216217
TString GetHTTPINTERNALERROR(const TRequestState& request, TString contentType = {}, TString response = {}) override;
217218
TString GetHTTPFORWARD(const TRequestState& request, const TString& location) override;
@@ -698,13 +699,19 @@ TString TViewer::GetHTTPBADREQUEST(const TRequestState& request, TString content
698699
return res;
699700
}
700701

701-
TString TViewer::GetHTTPFORBIDDEN(const TRequestState& request) {
702+
TString TViewer::GetHTTPFORBIDDEN(const TRequestState& request, TString contentType, TString response) {
702703
TStringBuilder res;
703704
res << "HTTP/1.1 403 Forbidden\r\n"
704705
<< "Connection: Close\r\n";
706+
if (contentType) {
707+
res << "Content-Type: " << contentType << "\r\n";
708+
}
705709
FillCORS(res, request);
706710
FillTraceId(res, request);
707711
res << "\r\n";
712+
if (response) {
713+
res << response;
714+
}
708715
return res;
709716
}
710717

@@ -767,6 +774,41 @@ TString TViewer::GetHTTPFORWARD(const TRequestState& request, const TString& loc
767774
return res;
768775
}
769776

777+
void MakeErrorReply(NJson::TJsonValue& jsonResponse, TString& message, const NYdb::TStatus& status) {
778+
google::protobuf::RepeatedPtrField<Ydb::Issue::IssueMessage> protoIssues;
779+
NYql::IssuesToMessage(status.GetIssues(), &protoIssues);
780+
781+
message.clear();
782+
// find first deepest error
783+
std::stable_sort(protoIssues.begin(), protoIssues.end(), [](const Ydb::Issue::IssueMessage& a, const Ydb::Issue::IssueMessage& b) -> bool {
784+
return a.severity() < b.severity();
785+
});
786+
787+
while (protoIssues.size() > 0 && protoIssues[0].issuesSize() > 0) {
788+
protoIssues = protoIssues[0].issues();
789+
}
790+
791+
if (protoIssues.size() > 0) {
792+
const Ydb::Issue::IssueMessage& issue = protoIssues[0];
793+
NProtobufJson::Proto2Json(issue, jsonResponse["error"]);
794+
message = issue.message();
795+
}
796+
797+
NJson::TJsonValue& jsonIssues = jsonResponse["issues"];
798+
for (const auto& queryIssue : protoIssues) {
799+
NJson::TJsonValue& issue = jsonIssues.AppendValue({});
800+
NProtobufJson::Proto2Json(queryIssue, issue);
801+
}
802+
803+
TString textStatus = TStringBuilder() << status.GetStatus();
804+
805+
jsonResponse["status"] = textStatus;
806+
807+
if (message.empty()) {
808+
message = textStatus;
809+
}
810+
}
811+
770812
NKikimrViewer::EFlag GetFlagFromTabletState(NKikimrWhiteboard::TTabletStateInfo::ETabletState state) {
771813
NKikimrViewer::EFlag flag = NKikimrViewer::EFlag::Grey;
772814
switch (state) {

‎ydb/core/viewer/viewer.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <ydb/core/driver_lib/run/config.h>
1212
#include <ydb/core/viewer/protos/viewer.pb.h>
1313
#include <ydb/public/api/protos/ydb_monitoring.pb.h>
14+
#include <ydb/public/sdk/cpp/client/ydb_types/status/status.h>
1415
#include <util/system/hostname.h>
1516

1617
namespace NKikimr {
@@ -194,7 +195,7 @@ class IViewer {
194195

195196
virtual TString GetHTTPGATEWAYTIMEOUT(const TRequestState& request, TString contentType = {}, TString response = {}) = 0;
196197
virtual TString GetHTTPBADREQUEST(const TRequestState& request, TString contentType = {}, TString response = {}) = 0;
197-
virtual TString GetHTTPFORBIDDEN(const TRequestState& request) = 0;
198+
virtual TString GetHTTPFORBIDDEN(const TRequestState& request, TString contentType = {}, TString response = {}) = 0;
198199
virtual TString GetHTTPNOTFOUND(const TRequestState& request) = 0;
199200
virtual TString GetHTTPINTERNALERROR(const TRequestState& request, TString contentType = {}, TString response = {}) = 0;
200201
virtual TString GetHTTPFORWARD(const TRequestState& request, const TString& location) = 0;
@@ -260,6 +261,8 @@ void SplitIds(TStringBuf source, char delim, std::unordered_set<ValueType>& valu
260261
GenericSplitIds<ValueType>(source, delim, std::inserter(values, values.end()));
261262
}
262263

264+
void MakeErrorReply(NJson::TJsonValue& jsonResponse, TString& message, const NYdb::TStatus& status);
265+
263266
TString GetHTTPOKJSON();
264267
TString GetHTTPGATEWAYTIMEOUT();
265268
NKikimrViewer::EFlag GetFlagFromTabletState(NKikimrWhiteboard::TTabletStateInfo::ETabletState state);

0 commit comments

Comments
 (0)