Skip to content

Commit

Permalink
change error format for http handlers (#6248)
Browse files Browse the repository at this point in the history
  • Loading branch information
@adameat
adameat authored Jul 4, 2024
1 parent a2023e5 commit 1064ddc
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 37 deletions.
12 changes: 8 additions & 4 deletions ydb/core/viewer/json_local_rpc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#include <ydb/library/actors/core/actor_bootstrapped.h>
#include <ydb/library/actors/core/mon.h>
#include <library/cpp/protobuf/json/json2proto.h>
#include <library/cpp/json/json_writer.h>
#include <ydb/core/base/tablet_pipe.h>
#include <ydb/library/services/services.pb.h>
#include <ydb/core/tx/schemeshard/schemeshard.h>
Expand Down Expand Up @@ -206,12 +207,15 @@ class TJsonLocalRpc : public TActorBootstrapped<TJsonLocalRpc<TProtoRequest, TPr
void ReplyAndPassAway() {
if (Result && Result->Status) {
if (!Result->Status->IsSuccess()) {
NJson::TJsonValue json;
TString message;
MakeErrorReply(json, message, Result->Status.value());
TStringStream stream;
NJson::WriteJson(&stream, &json);
if (Result->Status->GetStatus() == NYdb::EStatus::UNAUTHORIZED) {
return ReplyAndPassAway(Viewer->GetHTTPFORBIDDEN(Event->Get()));
return ReplyAndPassAway(Viewer->GetHTTPFORBIDDEN(Event->Get(), "application/json", stream.Str()));
} else {
TStringStream error;
Result->Status->Out(error);
return ReplyAndPassAway(Viewer->GetHTTPBADREQUEST(Event->Get(), "text/plain", error.Str()));
return ReplyAndPassAway(Viewer->GetHTTPBADREQUEST(Event->Get(), "application/json", stream.Str()));
}
} else {
TStringStream json;
Expand Down
46 changes: 16 additions & 30 deletions ydb/core/viewer/json_query.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,13 +398,14 @@ class TJsonQuery : public TViewerPipeClient<TJsonQuery> {
if (ev->Get()->Record.GetRef().GetYdbStatus() == Ydb::StatusIds::SUCCESS) {
QueryResponse.Set(std::move(ev));
MakeOkReply(jsonResponse, QueryResponse->Record.GetRef());
if (Schema == ESchemaType::Classic && Stats.empty() && (Action.empty() || Action == "execute")) {
jsonResponse = std::move(jsonResponse["result"]);
}
} else {
QueryResponse.Error("QueryError");
MakeErrorReply(jsonResponse, ev->Get()->Record.GetRef().MutableResponse()->MutableQueryIssues());
}

if (Schema == ESchemaType::Classic && Stats.empty() && (Action.empty() || Action == "execute")) {
jsonResponse = std::move(jsonResponse["result"]);
NYql::TIssues issues;
NYql::IssuesFromMessage(ev->Get()->Record.GetRef().GetResponse().GetQueryIssues(), issues);
MakeErrorReply(jsonResponse, NYdb::TStatus(NYdb::EStatus(ev->Get()->Record.GetRef().GetYdbStatus()), std::move(issues)));
}

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

TStringStream stream;
Expand Down Expand Up @@ -472,26 +475,11 @@ class TJsonQuery : public TViewerPipeClient<TJsonQuery> {
}

private:
void MakeErrorReply(NJson::TJsonValue& jsonResponse, google::protobuf::RepeatedPtrField<Ydb::Issue::IssueMessage>* protoIssues) {
NJson::TJsonValue& jsonIssues = jsonResponse["issues"];

// find first deepest error
std::stable_sort(protoIssues->begin(), protoIssues->end(), [](const Ydb::Issue::IssueMessage& a, const Ydb::Issue::IssueMessage& b) -> bool {
return a.severity() < b.severity();
});
while (protoIssues->size() > 0 && (*protoIssues)[0].issuesSize() > 0) {
protoIssues = (*protoIssues)[0].mutable_issues();
}
void MakeErrorReply(NJson::TJsonValue& jsonResponse, const NYdb::TStatus& status) {
TString message;
if (protoIssues->size() > 0) {
const Ydb::Issue::IssueMessage& issue = (*protoIssues)[0];
NProtobufJson::Proto2Json(issue, jsonResponse["error"]);
message = issue.message();
}
for (const auto& queryIssue : *protoIssues) {
NJson::TJsonValue& issue = jsonIssues.AppendValue({});
NProtobufJson::Proto2Json(queryIssue, issue);
}

NViewer::MakeErrorReply(jsonResponse, message, status);

if (Span) {
Span.EndError("Error");
}
Expand All @@ -513,11 +501,9 @@ class TJsonQuery : public TViewerPipeClient<TJsonQuery> {
}
}
catch (const std::exception& ex) {
google::protobuf::RepeatedPtrField<Ydb::Issue::IssueMessage> protoIssues;
Ydb::Issue::IssueMessage* issue = protoIssues.Add();
issue->set_message(TStringBuilder() << "Convert error: " << ex.what());
issue->set_severity(NYql::TSeverityIds::S_ERROR);
MakeErrorReply(jsonResponse, &protoIssues);
NYql::TIssues issues;
issues.AddIssue(TStringBuilder() << "Convert error: " << ex.what());
MakeErrorReply(jsonResponse, NYdb::TStatus(NYdb::EStatus::BAD_REQUEST, std::move(issues)));
return;
}
}
Expand Down
46 changes: 44 additions & 2 deletions ydb/core/viewer/viewer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <library/cpp/lwtrace/mon/mon_lwtrace.h>
#include <contrib/libs/yaml-cpp/include/yaml-cpp/yaml.h>
#include <library/cpp/yaml/as/tstring.h>
#include <library/cpp/protobuf/json/proto2json.h>
#include <util/system/fstat.h>
#include <util/stream/file.h>
#include "viewer.h"
Expand Down Expand Up @@ -211,7 +212,7 @@ class TViewer : public TActorBootstrapped<TViewer>, public IViewer {
TString GetHTTPOK(const TRequestState& request, TString type, TString response, TInstant lastModified) override;
TString GetHTTPGATEWAYTIMEOUT(const TRequestState& request, TString type, TString response) override;
TString GetHTTPBADREQUEST(const TRequestState& request, TString type, TString response) override;
TString GetHTTPFORBIDDEN(const TRequestState& request) override;
TString GetHTTPFORBIDDEN(const TRequestState& request, TString type, TString response) override;
TString GetHTTPNOTFOUND(const TRequestState& request) override;
TString GetHTTPINTERNALERROR(const TRequestState& request, TString contentType = {}, TString response = {}) override;
TString GetHTTPFORWARD(const TRequestState& request, const TString& location) override;
Expand Down Expand Up @@ -698,13 +699,19 @@ TString TViewer::GetHTTPBADREQUEST(const TRequestState& request, TString content
return res;
}

TString TViewer::GetHTTPFORBIDDEN(const TRequestState& request) {
TString TViewer::GetHTTPFORBIDDEN(const TRequestState& request, TString contentType, TString response) {
TStringBuilder res;
res << "HTTP/1.1 403 Forbidden\r\n"
<< "Connection: Close\r\n";
if (contentType) {
res << "Content-Type: " << contentType << "\r\n";
}
FillCORS(res, request);
FillTraceId(res, request);
res << "\r\n";
if (response) {
res << response;
}
return res;
}

Expand Down Expand Up @@ -767,6 +774,41 @@ TString TViewer::GetHTTPFORWARD(const TRequestState& request, const TString& loc
return res;
}

void MakeErrorReply(NJson::TJsonValue& jsonResponse, TString& message, const NYdb::TStatus& status) {
google::protobuf::RepeatedPtrField<Ydb::Issue::IssueMessage> protoIssues;
NYql::IssuesToMessage(status.GetIssues(), &protoIssues);

message.clear();
// find first deepest error
std::stable_sort(protoIssues.begin(), protoIssues.end(), [](const Ydb::Issue::IssueMessage& a, const Ydb::Issue::IssueMessage& b) -> bool {
return a.severity() < b.severity();
});

while (protoIssues.size() > 0 && protoIssues[0].issuesSize() > 0) {
protoIssues = protoIssues[0].issues();
}

if (protoIssues.size() > 0) {
const Ydb::Issue::IssueMessage& issue = protoIssues[0];
NProtobufJson::Proto2Json(issue, jsonResponse["error"]);
message = issue.message();
}

NJson::TJsonValue& jsonIssues = jsonResponse["issues"];
for (const auto& queryIssue : protoIssues) {
NJson::TJsonValue& issue = jsonIssues.AppendValue({});
NProtobufJson::Proto2Json(queryIssue, issue);
}

TString textStatus = TStringBuilder() << status.GetStatus();

jsonResponse["status"] = textStatus;

if (message.empty()) {
message = textStatus;
}
}

NKikimrViewer::EFlag GetFlagFromTabletState(NKikimrWhiteboard::TTabletStateInfo::ETabletState state) {
NKikimrViewer::EFlag flag = NKikimrViewer::EFlag::Grey;
switch (state) {
Expand Down
5 changes: 4 additions & 1 deletion ydb/core/viewer/viewer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <ydb/core/driver_lib/run/config.h>
#include <ydb/core/viewer/protos/viewer.pb.h>
#include <ydb/public/api/protos/ydb_monitoring.pb.h>
#include <ydb/public/sdk/cpp/client/ydb_types/status/status.h>
#include <util/system/hostname.h>

namespace NKikimr {
Expand Down Expand Up @@ -194,7 +195,7 @@ class IViewer {

virtual TString GetHTTPGATEWAYTIMEOUT(const TRequestState& request, TString contentType = {}, TString response = {}) = 0;
virtual TString GetHTTPBADREQUEST(const TRequestState& request, TString contentType = {}, TString response = {}) = 0;
virtual TString GetHTTPFORBIDDEN(const TRequestState& request) = 0;
virtual TString GetHTTPFORBIDDEN(const TRequestState& request, TString contentType = {}, TString response = {}) = 0;
virtual TString GetHTTPNOTFOUND(const TRequestState& request) = 0;
virtual TString GetHTTPINTERNALERROR(const TRequestState& request, TString contentType = {}, TString response = {}) = 0;
virtual TString GetHTTPFORWARD(const TRequestState& request, const TString& location) = 0;
Expand Down Expand Up @@ -260,6 +261,8 @@ void SplitIds(TStringBuf source, char delim, std::unordered_set<ValueType>& valu
GenericSplitIds<ValueType>(source, delim, std::inserter(values, values.end()));
}

void MakeErrorReply(NJson::TJsonValue& jsonResponse, TString& message, const NYdb::TStatus& status);

TString GetHTTPOKJSON();
TString GetHTTPGATEWAYTIMEOUT();
NKikimrViewer::EFlag GetFlagFromTabletState(NKikimrWhiteboard::TTabletStateInfo::ETabletState state);
Expand Down

0 comments on commit 1064ddc

Please sign in to comment.