Unsupported database type (DataStreams) by 403 http error (#930)

* Remove yexception and add try/catch

* Fix by pr comments

* Use {} init

Author: kardymonds

ydb/core/fq/libs/actors/database_resolver.cpp

@@ -138,6 +138,31 @@ class TResponseProcessor : public TActorBootstrapped<TResponseProcessor>
 
         LOG_T("ResponseProcessor::Handle(HttpIncomingResponse): got MDB API response: code=" << ev->Get()->Response->Status);
 
+        try {
+            HandleResponse(ev, requestIter, errorMessage, result);
+        } catch (...) {
+            const TString msg = TStringBuilder() << "error while response processing, params "
+                << ((requestIter != Requests.end()) ? requestIter->second.ToDebugString() : TString{"unknown"})
+                << ", details: " << CurrentExceptionMessage();
+            LOG_E("ResponseProccessor::Handle(TEvHttpIncomingResponse): " << msg);
+        }
+
+        LOG_T("ResponseProcessor::Handle(HttpIncomingResponse): progress: " 
+              << DatabaseId2Description.size() << " of " << Requests.size() << " requests are done");
+
+        if (HandledIds == Requests.size()) {
+            SendResolvedEndpointsAndDie(errorMessage);
+        }
+    }
+
+private:
+
+    void HandleResponse(
+        NHttp::TEvHttpProxy::TEvHttpIncomingResponse::TPtr& ev,
+        const TRequestMap::const_iterator& requestIter,
+        TString& errorMessage,
+        TMaybe<TDatabaseDescription>& result)
+    {
         if (ev->Get()->Error.empty() && (ev->Get()->Response && ev->Get()->Response->Status == "200")) {
             errorMessage = HandleSuccessfulResponse(ev, requestIter, result);
         } else {
@@ -152,34 +177,26 @@ class TResponseProcessor : public TActorBootstrapped<TResponseProcessor>
             auto key = std::make_tuple(params.Id, params.DatabaseType, params.DatabaseAuth);
             if (errorMessage) {
                 LOG_T("ResponseProcessor::Handle(HttpIncomingResponse): put value in cache"
-                      << "; params: " << params.ToDebugString()
-                      << ", error: " << errorMessage);
+                    << "; params: " << params.ToDebugString()
+                    << ", error: " << errorMessage);
                 Cache.Put(key, errorMessage);
             } else {
                 LOG_T("ResponseProcessor::Handle(HttpIncomingResponse): put value in cache"
-                      << "; params: " << params.ToDebugString()
-                      << ", result: " << result->ToDebugString());
+                    << "; params: " << params.ToDebugString()
+                    << ", result: " << result->ToDebugString());
                 Cache.Put(key, result);
             }
         }
-
-        LOG_D("ResponseProcessor::Handle(HttpIncomingResponse): progress: " 
-              << DatabaseId2Description.size() << " of " << Requests.size() << " requests are done");
-
-        if (HandledIds == Requests.size()) {
-            SendResolvedEndpointsAndDie(errorMessage);
-        }
     }
 
-private:
     TString HandleSuccessfulResponse(
         NHttp::TEvHttpProxy::TEvHttpIncomingResponse::TPtr& ev,
         const TRequestMap::const_iterator& requestIter,
         TMaybe<TDatabaseDescription>& result
     ) {
         if (requestIter == Requests.end()) {
             return "unknown request";
-        } 
+        }
 
         NJson::TJsonReaderConfig jsonConfig;
         NJson::TJsonValue databaseInfo;
@@ -224,12 +241,7 @@ class TResponseProcessor : public TActorBootstrapped<TResponseProcessor>
         const auto& status = ev->Get()->Response->Status;
 
         if (status == "403") {
-            const auto second = requestIter->second;
-            auto mdbTypeStr = NYql::DatabaseTypeLowercase(second.DatabaseType);
-
-            return TStringBuilder() << "You have no permission to resolve database id into database endpoint. " <<
-                                       "Please check that your service account has role "  << 
-                                       "`managed-" << mdbTypeStr << ".viewer`.";
+            return TStringBuilder() << "You have no permission to resolve database id into database endpoint. " + DetailedPermissionsError(requestIter->second);
         }
 
         auto errorMessage = ev->Get()->Error;
@@ -245,6 +257,17 @@ class TResponseProcessor : public TActorBootstrapped<TResponseProcessor>
         return errorMessage;
     }
 
+
+    TString DetailedPermissionsError(const TResolveParams& params) const {
+ 
+        if (params.DatabaseType == EDatabaseType::ClickHouse || params.DatabaseType == EDatabaseType::PostgreSQL) {
+                auto mdbTypeStr = NYql::DatabaseTypeLowercase(params.DatabaseType);
+                return TStringBuilder() << "Please check that your service account has role "  << 
+                                       "`managed-" << mdbTypeStr << ".viewer`.";
+        }
+        return {};
+    }
+
     const TActorId Sender;
     TCache& Cache;
     const TRequestMap Requests;

ydb/core/fq/libs/actors/ut/database_resolver_ut.cpp

@@ -14,6 +14,8 @@ namespace {
 using namespace NKikimr;
 using namespace NFq;
 
+TString NoPermissionStr = "You have no permission to resolve database id into database endpoint. ";
+
 struct TTestBootstrap : public TTestActorRuntime {
     NConfig::TCheckpointCoordinatorConfig Settings;
     NActors::TActorId DatabaseResolver;
@@ -295,7 +297,7 @@ Y_UNIT_TEST_SUITE(TDatabaseResolverTests) {
     Y_UNIT_TEST(ClickHouse_PermissionDenied) {
         NYql::TIssues issues{
             NYql::TIssue(
-                "You have no permission to resolve database id into database endpoint. Please check that your service account has role `managed-clickhouse.viewer`."
+                TStringBuilder{} << NoPermissionStr << "Please check that your service account has role `managed-clickhouse.viewer`."
             )
         };
 
@@ -363,7 +365,7 @@ Y_UNIT_TEST_SUITE(TDatabaseResolverTests) {
     Y_UNIT_TEST(PostgreSQL_PermissionDenied) {
         NYql::TIssues issues{
             NYql::TIssue(
-                "You have no permission to resolve database id into database endpoint. Please check that your service account has role `managed-postgresql.viewer`."
+                TStringBuilder{} << NoPermissionStr << "Please check that your service account has role `managed-postgresql.viewer`."
             )
         };
 
@@ -389,6 +391,27 @@ Y_UNIT_TEST_SUITE(TDatabaseResolverTests) {
                 issues
             );
     }
+
+    Y_UNIT_TEST(DataStreams_PermissionDenied) {
+        NYql::TIssues issues{
+            NYql::TIssue(
+                NoPermissionStr
+            )
+        };
+        Test(
+            NYql::EDatabaseType::DataStreams,
+            NYql::NConnector::NApi::EProtocol::PROTOCOL_UNSPECIFIED,
+            "https://ydbc.ydb.cloud.yandex.net:8789/ydbc/cloud-prod/database?databaseId=etn021us5r9rhld1vgbh",
+            "403",
+            R"(
+                {
+                    "message": "Permission denied"
+                })",
+            NYql::TDatabaseResolverResponse::TDatabaseDescription{
+                },
+                issues
+            );
+    }
 }
 
 } // namespace NFq

ydb/library/yql/providers/common/db_id_async_resolver/db_async_resolver.h

@@ -41,14 +41,7 @@ inline NConnector::NApi::EDataSourceKind DatabaseTypeToDataSourceKind(EDatabaseT
 inline TString DatabaseTypeLowercase(EDatabaseType databaseType) {
     auto dump = ToString(databaseType);
     dump.to_lower();
-
-    switch (databaseType) {
-        case EDatabaseType::ClickHouse:
-        case EDatabaseType::PostgreSQL:
-            return dump;
-        default:
-            ythrow yexception() << "Unsupported database type: " << ToString(databaseType);
-    }
+    return dump;
 }
 
 // TODO: remove this function after /kikimr/yq/tests/control_plane_storage is moved to /ydb.
@@ -73,7 +66,7 @@ struct TDatabaseAuth {
     NConnector::NApi::EProtocol Protocol = NConnector::NApi::EProtocol::PROTOCOL_UNSPECIFIED;
 
     bool operator==(const TDatabaseAuth& other) const {
-        return std::tie(StructuredToken, AddBearerToToken, UseTls) == std::tie(other.StructuredToken, other.AddBearerToToken, other.UseTls);
+        return std::tie(StructuredToken, AddBearerToToken, UseTls, Protocol) == std::tie(other.StructuredToken, other.AddBearerToToken, other.UseTls, Protocol);
     }
 
     bool operator!=(const TDatabaseAuth& other) const {