Skip to content

change error format for http handlers #6248

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions ydb/core/viewer/json_local_rpc.h
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
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
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
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
Loading