improve error handling (#5974)

ydb-platform · Jun 27, 2024 · f400427 · f400427
1 parent bb4992d
commit f400427
Show file tree

Hide file tree

Showing 11 changed files with 99 additions and 11 deletions.
diff --git a/ydb/library/yql/providers/solomon/async_io/dq_solomon_read_actor.cpp b/ydb/library/yql/providers/solomon/async_io/dq_solomon_read_actor.cpp
@@ -337,22 +337,20 @@ class TDqSolomonReadActor : public NActors::TActorBootstrapped<TDqSolomonReadAct
     }
 
     void Handle(TEvHttpBase::TEvSendResult::TPtr& ev) {
-        //Cerr << "EX: Handle(TEvHttpBase::TEvSendResult::TPtr& ev)" << Endl;
         const auto* res = ev->Get();
         const TString& error = res->HttpIncomingResponse->Get()->GetError();
 
-        //Cerr << "EX: Handle(TEvHttpBase::TEvSendResult::TPtr& ev), error: " << error << Endl;
         if (!error.empty() || (res->HttpIncomingResponse->Get()->Response && res->HttpIncomingResponse->Get()->Response->Status != "200")) {
             TStringBuilder errorBuilder;
             errorBuilder << "Error while sending request to monitoring api: " << error;
             const auto& response = res->HttpIncomingResponse->Get()->Response;
             if (response) {
-                errorBuilder << " " << response->GetObfuscatedData();
+                errorBuilder << ", response: " << response->Body.Head(10000);
             }
 
             TIssues issues { TIssue(errorBuilder) };
             SINK_LOG_W("Got " << (res->IsTerminal ? "terminal " : "") << "error response[" << ev->Cookie << "] from solomon: " << issues.ToOneLineString());
-            //Cerr << "Got " << (res->IsTerminal ? "terminal " : "") << "error response[" << ev->Cookie << "] from solomon: " << issues.ToOneLineString();
+            Send(ComputeActorId, new TEvAsyncInputError(InputIndex, issues, NYql::NDqProto::StatusIds::EXTERNAL_ERROR));
             return;
         }
 
@@ -361,12 +359,13 @@ class TDqSolomonReadActor : public NActors::TActorBootstrapped<TDqSolomonReadAct
 
     void HandleSuccessSolomonResponse(const NHttp::TEvHttpProxy::TEvHttpIncomingResponse& response, ui64 cookie) {
         Y_UNUSED(cookie);
-        //SINK_LOG_E("Solomon response[" << cookie << "]: " << response.Response->GetObfuscatedData());
-        //Cerr << "EX:" << response.Response->Body << Endl;
         NJson::TJsonValue json;
-        if (!NJson::ReadJsonTree(response.Response->Body, &json, false)) {
-            // todo: improve
-            Y_ABORT_UNLESS(false, "Failed to parse json response");
+        try {
+            NJson::ReadJsonTree(response.Response->Body, &json, /*throwOnError*/ true);
+        } catch (const std::exception& e) {
+            SINK_LOG_E("Invalid JSON reponse from monitoring: " << e.what() << ", body: " << response.Response->Body.Head(10000));
+            TIssues issues { TIssue(TStringBuilder() << "Failed to parse response from monitoring: " << e.what()) };
+            Send(ComputeActorId, new TEvAsyncInputError(InputIndex, issues, NYql::NDqProto::StatusIds::EXTERNAL_ERROR));
             return;
         }
 

diff --git a/ydb/library/yql/tests/sql/solomon/canondata/result.json b/ydb/library/yql/tests/sql/solomon/canondata/result.json
@@ -30,6 +30,11 @@
             "uri": "file://test.test_solomon-Basic-default.txt_/plan.txt"
         }
     ],
+    "test.test[solomon-BrokenJsonResponse-]": [
+        {
+            "uri": "file://test.test_solomon-BrokenJsonResponse-_/extracted"
+        }
+    ],
     "test.test[solomon-Downsampling-default.txt]": [
         {
             "uri": "file://test.test_solomon-Downsampling-default.txt_/results.txt"
@@ -52,6 +57,11 @@
             "uri": "file://test.test_solomon-DownsamplingValidSettings-default.txt_/plan.txt"
         }
     ],
+    "test.test[solomon-InvalidProject-]": [
+        {
+            "uri": "file://test.test_solomon-InvalidProject-_/extracted"
+        }
+    ],
     "test.test[solomon-LabelColumns-default.txt]": [
         {
             "uri": "file://test.test_solomon-LabelColumns-default.txt_/results.txt"

diff --git a/ydb/library/yql/tests/sql/solomon/canondata/test.test_solomon-BrokenJsonResponse-_/extracted b/ydb/library/yql/tests/sql/solomon/canondata/test.test_solomon-BrokenJsonResponse-_/extracted
@@ -0,0 +1,9 @@
+<tmp_path>/program.sql:<main>: Error: Execution
+
+    <tmp_path>/program.sql:<main>:1:1: Warning: Gateway Error
+    	SELECT * FROM local_solomon.broken_json WITH (
+	^
+    <tmp_path>/program.sql:<main>: Error: Fatal Error
+
+
+        <tmp_path>/program.sql:<main>: Error: Failed to parse response from monitoring: library/cpp/json/json_reader.cpp:417: Offset: 2, Code: 4, Error: Missing a name for object member. (appeared 1 time at ISOTIME)
diff --git a/ydb/library/yql/tests/sql/solomon/canondata/test.test_solomon-InvalidProject-_/extracted b/ydb/library/yql/tests/sql/solomon/canondata/test.test_solomon-InvalidProject-_/extracted
@@ -0,0 +1,9 @@
+<tmp_path>/program.sql:<main>: Error: Execution
+
+    <tmp_path>/program.sql:<main>:1:1: Warning: Gateway Error
+    	SELECT * FROM local_solomon.invalid WITH (
+	^
+    <tmp_path>/program.sql:<main>: Error: Fatal Error
+
+
+        <tmp_path>/program.sql:<main>: Error: Error while sending request to monitoring api: 404 Not Found, response: Project invalid does not exist (appeared 1 time at ISOTIME)
diff --git a/ydb/library/yql/tests/sql/solomon/test.py b/ydb/library/yql/tests/sql/solomon/test.py
@@ -1,5 +1,6 @@
 import pytest
 import os
+import re
 import codecs
 import yatest.common
 
@@ -24,6 +25,17 @@
 DATA_PATH = yatest.common.source_path('ydb/library/yql/tests/sql/suites')
 
 
+def read_file(path):
+    with open(path, "r") as f:
+        return f.read()
+
+
+def normalize_timestamp_string(s):
+    # 2022-08-13T16:11:21Z -> ISOTIME
+    # 2022-08-13T16:11:21.549879Z -> ISOTIME
+    return re.sub(r"2\d{3}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(.\d+)?Z", "ISOTIME", s)
+
+
 def pytest_generate_tests(metafunc):
     return pytest_generate_tests_for_run(metafunc, suites=['solomon'], data_path=DATA_PATH)
 
@@ -56,7 +68,8 @@ def test(suite, case, cfg, solomon):
 
     files = get_files(suite, config, DATA_PATH)
 
-    yqlrun = YQLRun(prov=prov, gateway_config=compose_gateways_config(solomon.endpoint), binary=DQRUN_PATH, extra_args=["--emulate-yt"])
+    yqlrun = YQLRun(prov=prov, gateway_config=compose_gateways_config(solomon.endpoint), binary=DQRUN_PATH,
+                    extra_args=["--emulate-yt"])
     yqlrun_res = yqlrun.yql_exec(
         program=sql_query,
         run_sql=True,
@@ -67,7 +80,7 @@ def test(suite, case, cfg, solomon):
 
     if xfail:
         assert yqlrun_res.execution_result.exit_code != 0
-        return [normalize_source_code_path(yqlrun_res.std_err)]
+        return [normalize_source_code_path(normalize_timestamp_string(yqlrun_res.std_err))]
 
     return [yatest.common.canonical_file(yqlrun_res.results_file, local=True),
             yatest.common.canonical_file(yqlrun_res.opt_file, local=True, diff_tool=ASTDIFF_PATH),

diff --git a/ydb/library/yql/tests/sql/sql2yql/canondata/result.json b/ydb/library/yql/tests/sql/sql2yql/canondata/result.json
@@ -17128,6 +17128,13 @@
             "uri": "https://{canondata_backend}/1880306/2acf5b55cb208565b2cb35901ac765fc11dca857/resource.tar.gz#test_sql2yql.test_solomon-Basic_/sql.yql"
         }
     ],
+    "test_sql2yql.test[solomon-BrokenJsonResponse]": [
+        {
+            "checksum": "3989d4c834dd77b2601f4c031d062837",
+            "size": 1199,
+            "uri": "https://{canondata_backend}/1599023/688233977db70f339a250f6f25033e36d5b327fb/resource.tar.gz#test_sql2yql.test_solomon-BrokenJsonResponse_/sql.yql"
+        }
+    ],
     "test_sql2yql.test[solomon-DownsamplingValidSettings]": [
         {
             "checksum": "1e75df3c5096dd36a54c18e633a607a1",
@@ -17142,6 +17149,13 @@
             "uri": "https://{canondata_backend}/1880306/2acf5b55cb208565b2cb35901ac765fc11dca857/resource.tar.gz#test_sql2yql.test_solomon-Downsampling_/sql.yql"
         }
     ],
+    "test_sql2yql.test[solomon-InvalidProject]": [
+        {
+            "checksum": "a245f9de278996da0a0fb71395f5bdf0",
+            "size": 1195,
+            "uri": "https://{canondata_backend}/1599023/688233977db70f339a250f6f25033e36d5b327fb/resource.tar.gz#test_sql2yql.test_solomon-InvalidProject_/sql.yql"
+        }
+    ],
     "test_sql2yql.test[solomon-LabelColumns]": [
         {
             "checksum": "9cbee7499e54e16143791b73d4891146",
@@ -33319,6 +33333,13 @@
             "uri": "https://{canondata_backend}/1880306/2acf5b55cb208565b2cb35901ac765fc11dca857/resource.tar.gz#test_sql_format.test_solomon-Basic_/formatted.sql"
         }
     ],
+    "test_sql_format.test[solomon-BrokenJsonResponse]": [
+        {
+            "checksum": "2ea7d86f378d1d0acea749729505b8e9",
+            "size": 133,
+            "uri": "https://{canondata_backend}/1599023/688233977db70f339a250f6f25033e36d5b327fb/resource.tar.gz#test_sql_format.test_solomon-BrokenJsonResponse_/formatted.sql"
+        }
+    ],
     "test_sql_format.test[solomon-DownsamplingValidSettings]": [
         {
             "checksum": "1741d8968029a8b3c23885c5826009f3",
@@ -33333,6 +33354,13 @@
             "uri": "https://{canondata_backend}/1880306/2acf5b55cb208565b2cb35901ac765fc11dca857/resource.tar.gz#test_sql_format.test_solomon-Downsampling_/formatted.sql"
         }
     ],
+    "test_sql_format.test[solomon-InvalidProject]": [
+        {
+            "checksum": "d9dc0179f957667ff446aae66459f359",
+            "size": 129,
+            "uri": "https://{canondata_backend}/1599023/688233977db70f339a250f6f25033e36d5b327fb/resource.tar.gz#test_sql_format.test_solomon-InvalidProject_/formatted.sql"
+        }
+    ],
     "test_sql_format.test[solomon-LabelColumns]": [
         {
             "checksum": "5d2ff6d76dac9a34c9ca42f73fedbc40",

diff --git a/ydb/library/yql/tests/sql/suites/solomon/BrokenJsonResponse.cfg b/ydb/library/yql/tests/sql/suites/solomon/BrokenJsonResponse.cfg
@@ -0,0 +1,2 @@
+providers solomon
+xfail
diff --git a/ydb/library/yql/tests/sql/suites/solomon/BrokenJsonResponse.sql b/ydb/library/yql/tests/sql/suites/solomon/BrokenJsonResponse.sql
@@ -0,0 +1,5 @@
+SELECT * FROM local_solomon.broken_json WITH (
+  program = @@{}@@,
+  from = "2023-12-08T14:40:39Z",
+  to = "2023-12-08T14:45:39Z"
+);
diff --git a/ydb/library/yql/tests/sql/suites/solomon/InvalidProject.cfg b/ydb/library/yql/tests/sql/suites/solomon/InvalidProject.cfg
@@ -0,0 +1,2 @@
+providers solomon
+xfail
diff --git a/ydb/library/yql/tests/sql/suites/solomon/InvalidProject.sql b/ydb/library/yql/tests/sql/suites/solomon/InvalidProject.sql
@@ -0,0 +1,5 @@
+SELECT * FROM local_solomon.invalid WITH (
+  program = @@{}@@,
+  from = "2023-12-08T14:40:39Z",
+  to = "2023-12-08T14:45:39Z"
+);
diff --git a/ydb/library/yql/tools/solomon_emulator/lib/webapp.py b/ydb/library/yql/tools/solomon_emulator/lib/webapp.py
@@ -126,6 +126,12 @@ def _dict_to_labels(body):
 @routes.post("/api/v2/projects/{project}/sensors/data")
 async def sensors_data(request):
     project = request.match_info["project"]
+    if project == "invalid":
+        return web.HTTPNotFound(text=f"Project {project} does not exist")
+
+    if project == "broken_json":
+        return web.Response(text="{ broken json", content_type="application/json")
+
     labels = _dict_to_labels(await request.json())
     labels["project"] = project
     return web.json_response({"vector": [