Merge pull request #61951 from Avogar/fix-exception-in-format

Fix writing exception message in output format in HTTP when http_wait_end_of_query is used

src/Interpreters/executeQuery.cpp

@@ -1453,6 +1453,7 @@ void executeQuery(
     ASTPtr ast;
     BlockIO streams;
     OutputFormatPtr output_format;
+    String format_name;
 
     auto update_format_on_exception_if_needed = [&]()
     {
@@ -1460,7 +1461,7 @@ void executeQuery(
         {
             try
             {
-                String format_name = context->getDefaultFormat();
+                format_name = context->getDefaultFormat();
                 output_format = FormatFactory::instance().getOutputFormat(format_name, ostr, {}, context, output_format_settings);
                 if (output_format && output_format->supportsWritingException())
                 {
@@ -1501,7 +1502,7 @@ void executeQuery(
         {
             update_format_on_exception_if_needed();
             if (output_format)
-                handle_exception_in_output_format(*output_format);
+                handle_exception_in_output_format(*output_format, format_name, context, output_format_settings);
         }
         throw;
     }
@@ -1543,7 +1544,7 @@ void executeQuery(
                 );
             }
 
-            String format_name = ast_query_with_output && (ast_query_with_output->format != nullptr)
+            format_name = ast_query_with_output && (ast_query_with_output->format != nullptr)
                                     ? getIdentifierName(ast_query_with_output->format)
                                     : context->getDefaultFormat();
 
@@ -1609,7 +1610,7 @@ void executeQuery(
         {
             update_format_on_exception_if_needed();
             if (output_format)
-                handle_exception_in_output_format(*output_format);
+                handle_exception_in_output_format(*output_format, format_name, context, output_format_settings);
         }
         throw;
     }

src/Interpreters/executeQuery.h

@@ -27,7 +27,7 @@ struct QueryResultDetails
 };
 
 using SetResultDetailsFunc = std::function<void(const QueryResultDetails &)>;
-using HandleExceptionInOutputFormatFunc = std::function<void(IOutputFormat & output_format)>;
+using HandleExceptionInOutputFormatFunc = std::function<void(IOutputFormat & output_format, const String & format_name, const ContextPtr & context, const std::optional<FormatSettings> & format_settings)>;
 
 struct QueryFlags
 {

src/Server/HTTPHandler.cpp

@@ -873,19 +873,34 @@ void HTTPHandler::processQuery(
             response.add("X-ClickHouse-Timezone", *details.timezone);
     };
 
-    auto handle_exception_in_output_format = [&](IOutputFormat & output_format)
+    auto handle_exception_in_output_format = [&](IOutputFormat & current_output_format, const String & format_name, const ContextPtr & context_, const std::optional<FormatSettings> & format_settings)
     {
-        if (settings.http_write_exception_in_output_format && output_format.supportsWritingException())
+        if (settings.http_write_exception_in_output_format && current_output_format.supportsWritingException())
+        {
+            /// If wait_end_of_query=true in case of an exception all data written to output format during query execution will be
+            /// ignored, so we cannot write exception message in current output format as it will be also ignored.
+            /// Instead, we create exception_writer function that will write exception in required format
+            /// and will use it later in trySendExceptionToClient when all buffers will be prepared.
+            if (buffer_until_eof)
+            {
+                auto header = current_output_format.getPort(IOutputFormat::PortKind::Main).getHeader();
+                used_output.exception_writer = [format_name, header, context_, format_settings](WriteBuffer & buf, const String & message)
+                {
+                    auto output_format = FormatFactory::instance().getOutputFormat(format_name, buf, header, context_, format_settings);
+                    output_format->setException(message);
+                    output_format->finalize();
+                };
+            }
+            else
             {
                 bool with_stacktrace = (params.getParsed<bool>("stacktrace", false) && server.config().getBool("enable_http_stacktrace", true));
-
                 ExecutionStatus status = ExecutionStatus::fromCurrentException("", with_stacktrace);
                 formatExceptionForClient(status.code, request, response, used_output);
-
+                current_output_format.setException(status.message);
-            output_format.setException(getCurrentExceptionMessage(false));
+                current_output_format.finalize();
-            output_format.finalize();
                 used_output.exception_is_written = true;
             }
+        }
     };
 
     executeQuery(
@@ -948,9 +963,17 @@ try
                 used_output.out_holder->position() = used_output.out_holder->buffer().begin();
             }
 
+            /// We might have special formatter for exception message.
+            if (used_output.exception_writer)
+            {
+                used_output.exception_writer(*used_output.out_maybe_compressed, s);
+            }
+            else
+            {
                 writeString(s, *used_output.out_maybe_compressed);
                 writeChar('\n', *used_output.out_maybe_compressed);
             }
+        }
 
         used_output.out_maybe_compressed->next();
     }

src/Server/HTTPHandler.h

@@ -75,6 +75,7 @@ private:
         bool finalized = false;
 
         bool exception_is_written = false;
+        std::function<void(WriteBuffer &, const String &)> exception_writer;
 
         inline bool hasDelayed() const
         {

tests/queries/0_stateless/02841_valid_json_and_xml_on_http_exception.reference

@@ -1,3 +1,4 @@
+wait_end_of_query=0
 One block
 Parallel formatting: 0
 JSON
@@ -430,3 +431,350 @@ Test 2
 Test 3
 1
 1
+wait_end_of_query=1
+One block
+Parallel formatting: 0
+JSON
+{
+	"meta":
+	[
+		{
+			"name": "number",
+			"type": "UInt64"
+		},
+		{
+			"name": "res",
+			"type": "UInt8"
+		}
+	],
+
+	"data":
+	[
+
+	],
+
+	"rows": 0,
+
+	"exception": "Code: 395. : Value passed to 'throwIf' function is non-zero: while executing 'FUNCTION throwIf(greater(number, 3) :: 2) -> throwIf(greater(number, 3)) UInt8 : 1'. (FUNCTION_THROW_IF_VALUE_IS_NON_ZERO) "
+}
+JSONEachRow
+{"exception": "Code: 395. : Value passed to 'throwIf' function is non-zero: while executing 'FUNCTION throwIf(greater(number, 3) :: 2) -> throwIf(greater(number, 3)) UInt8 : 1'. (FUNCTION_THROW_IF_VALUE_IS_NON_ZERO) "}
+JSONCompact
+{
+	"meta":
+	[
+		{
+			"name": "number",
+			"type": "UInt64"
+		},
+		{
+			"name": "res",
+			"type": "UInt8"
+		}
+	],
+
+	"data":
+	[
+
+	],
+
+	"rows": 0,
+
+	"exception": "Code: 395. : Value passed to 'throwIf' function is non-zero: while executing 'FUNCTION throwIf(greater(number, 3) :: 2) -> throwIf(greater(number, 3)) UInt8 : 1'. (FUNCTION_THROW_IF_VALUE_IS_NON_ZERO) "
+}
+JSONCompactEachRow
+["Code: 395. : Value passed to 'throwIf' function is non-zero: while executing 'FUNCTION throwIf(greater(number, 3) :: 2) -> throwIf(greater(number, 3)) UInt8 : 1'. (FUNCTION_THROW_IF_VALUE_IS_NON_ZERO) "]
+JSONObjectEachRow
+{
+	"exception": "Code: 395. : Value passed to 'throwIf' function is non-zero: while executing 'FUNCTION throwIf(greater(number, 3) :: 2) -> throwIf(greater(number, 3)) UInt8 : 1'. (FUNCTION_THROW_IF_VALUE_IS_NON_ZERO) "
+}
+XML
+<?xml version='1.0' encoding='UTF-8' ?>
+<result>
+	<meta>
+		<columns>
+			<column>
+				<name>number</name>
+				<type>UInt64</type>
+			</column>
+			<column>
+				<name>res</name>
+				<type>UInt8</type>
+			</column>
+		</columns>
+	</meta>
+	<data>
+	</data>
+	<rows>0</rows>
+	<exception>Code: 395. : Value passed to 'throwIf' function is non-zero: while executing 'FUNCTION throwIf(greater(number, 3) :: 2) -> throwIf(greater(number, 3)) UInt8 : 1'. (FUNCTION_THROW_IF_VALUE_IS_NON_ZERO) </exception>
+</result>
+Parallel formatting: 1
+JSON
+{
+	"meta":
+	[
+		{
+			"name": "number",
+			"type": "UInt64"
+		},
+		{
+			"name": "res",
+			"type": "UInt8"
+		}
+	],
+
+	"data":
+	[
+
+	],
+
+	"rows": 0,
+
+	"exception": "Code: 395. : Value passed to 'throwIf' function is non-zero: while executing 'FUNCTION throwIf(greater(number, 3) :: 2) -> throwIf(greater(number, 3)) UInt8 : 1'. (FUNCTION_THROW_IF_VALUE_IS_NON_ZERO) "
+}
+JSONEachRow
+{"exception": "Code: 395. : Value passed to 'throwIf' function is non-zero: while executing 'FUNCTION throwIf(greater(number, 3) :: 2) -> throwIf(greater(number, 3)) UInt8 : 1'. (FUNCTION_THROW_IF_VALUE_IS_NON_ZERO) "}
+JSONCompact
+{
+	"meta":
+	[
+		{
+			"name": "number",
+			"type": "UInt64"
+		},
+		{
+			"name": "res",
+			"type": "UInt8"
+		}
+	],
+
+	"data":
+	[
+
+	],
+
+	"rows": 0,
+
+	"exception": "Code: 395. : Value passed to 'throwIf' function is non-zero: while executing 'FUNCTION throwIf(greater(number, 3) :: 2) -> throwIf(greater(number, 3)) UInt8 : 1'. (FUNCTION_THROW_IF_VALUE_IS_NON_ZERO) "
+}
+JSONCompactEachRow
+["Code: 395. : Value passed to 'throwIf' function is non-zero: while executing 'FUNCTION throwIf(greater(number, 3) :: 2) -> throwIf(greater(number, 3)) UInt8 : 1'. (FUNCTION_THROW_IF_VALUE_IS_NON_ZERO) "]
+JSONObjectEachRow
+{
+	"exception": "Code: 395. : Value passed to 'throwIf' function is non-zero: while executing 'FUNCTION throwIf(greater(number, 3) :: 2) -> throwIf(greater(number, 3)) UInt8 : 1'. (FUNCTION_THROW_IF_VALUE_IS_NON_ZERO) "
+}
+XML
+<?xml version='1.0' encoding='UTF-8' ?>
+<result>
+	<meta>
+		<columns>
+			<column>
+				<name>number</name>
+				<type>UInt64</type>
+			</column>
+			<column>
+				<name>res</name>
+				<type>UInt8</type>
+			</column>
+		</columns>
+	</meta>
+	<data>
+	</data>
+	<rows>0</rows>
+	<exception>Code: 395. : Value passed to 'throwIf' function is non-zero: while executing 'FUNCTION throwIf(greater(number, 3) :: 2) -> throwIf(greater(number, 3)) UInt8 : 1'. (FUNCTION_THROW_IF_VALUE_IS_NON_ZERO) </exception>
+</result>
+Several blocks
+Without parallel formatting
+JSON
+{
+	"meta":
+	[
+		{
+			"name": "number",
+			"type": "UInt64"
+		},
+		{
+			"name": "res",
+			"type": "UInt8"
+		}
+	],
+
+	"data":
+	[
+
+	],
+
+	"rows": 0,
+
+	"exception": "Code: 395. : Value passed to 'throwIf' function is non-zero: while executing 'FUNCTION throwIf(greater(number, 3) :: 2) -> throwIf(greater(number, 3)) UInt8 : 1'. (FUNCTION_THROW_IF_VALUE_IS_NON_ZERO) "
+}
+JSONEachRow
+{"exception": "Code: 395. : Value passed to 'throwIf' function is non-zero: while executing 'FUNCTION throwIf(greater(number, 3) :: 2) -> throwIf(greater(number, 3)) UInt8 : 1'. (FUNCTION_THROW_IF_VALUE_IS_NON_ZERO) "}
+JSONCompact
+{
+	"meta":
+	[
+		{
+			"name": "number",
+			"type": "UInt64"
+		},
+		{
+			"name": "res",
+			"type": "UInt8"
+		}
+	],
+
+	"data":
+	[
+
+	],
+
+	"rows": 0,
+
+	"exception": "Code: 395. : Value passed to 'throwIf' function is non-zero: while executing 'FUNCTION throwIf(greater(number, 3) :: 2) -> throwIf(greater(number, 3)) UInt8 : 1'. (FUNCTION_THROW_IF_VALUE_IS_NON_ZERO) "
+}
+JSONCompactEachRow
+["Code: 395. : Value passed to 'throwIf' function is non-zero: while executing 'FUNCTION throwIf(greater(number, 3) :: 2) -> throwIf(greater(number, 3)) UInt8 : 1'. (FUNCTION_THROW_IF_VALUE_IS_NON_ZERO) "]
+JSONObjectEachRow
+{
+	"exception": "Code: 395. : Value passed to 'throwIf' function is non-zero: while executing 'FUNCTION throwIf(greater(number, 3) :: 2) -> throwIf(greater(number, 3)) UInt8 : 1'. (FUNCTION_THROW_IF_VALUE_IS_NON_ZERO) "
+}
+XML
+<?xml version='1.0' encoding='UTF-8' ?>
+<result>
+	<meta>
+		<columns>
+			<column>
+				<name>number</name>
+				<type>UInt64</type>
+			</column>
+			<column>
+				<name>res</name>
+				<type>UInt8</type>
+			</column>
+		</columns>
+	</meta>
+	<data>
+	</data>
+	<rows>0</rows>
+	<exception>Code: 395. : Value passed to 'throwIf' function is non-zero: while executing 'FUNCTION throwIf(greater(number, 3) :: 2) -> throwIf(greater(number, 3)) UInt8 : 1'. (FUNCTION_THROW_IF_VALUE_IS_NON_ZERO) </exception>
+</result>
+With parallel formatting
+JSON
+1
+JSONCompact
+1
+JSONObjectEachRow
+1
+JSONEachRow
+1
+JSONCompactEachRow
+1
+Formatting error
+Without parallel formatting
+JSON
+{
+	"meta":
+	[
+		{
+			"name": "x",
+			"type": "UInt32"
+		},
+		{
+			"name": "s",
+			"type": "String"
+		},
+		{
+			"name": "y",
+			"type": "Enum8('a' = 1)"
+		}
+	],
+
+	"data":
+	[
+
+	],
+
+	"rows": 0,
+
+	"exception": "Code: 36. : Unexpected value 99 in enum: While executing JSONRowOutputFormat. (BAD_ARGUMENTS) "
+}
+JSONEachRow
+{"exception": "Code: 36. : Unexpected value 99 in enum: While executing JSONEachRowRowOutputFormat. (BAD_ARGUMENTS) "}
+JSONCompact
+{
+	"meta":
+	[
+		{
+			"name": "x",
+			"type": "UInt32"
+		},
+		{
+			"name": "s",
+			"type": "String"
+		},
+		{
+			"name": "y",
+			"type": "Enum8('a' = 1)"
+		}
+	],
+
+	"data":
+	[
+
+	],
+
+	"rows": 0,
+
+	"exception": "Code: 36. : Unexpected value 99 in enum: While executing JSONCompactRowOutputFormat. (BAD_ARGUMENTS) "
+}
+JSONCompactEachRow
+["Code: 36. : Unexpected value 99 in enum: While executing JSONCompactEachRowRowOutputFormat. (BAD_ARGUMENTS) "]
+JSONObjectEachRow
+{
+	"exception": "Code: 36. : Unexpected value 99 in enum: While executing JSONObjectEachRowRowOutputFormat. (BAD_ARGUMENTS) "
+}
+XML
+<?xml version='1.0' encoding='UTF-8' ?>
+<result>
+	<meta>
+		<columns>
+			<column>
+				<name>x</name>
+				<type>UInt32</type>
+			</column>
+			<column>
+				<name>s</name>
+				<type>String</type>
+			</column>
+			<column>
+				<name>y</name>
+				<type>Enum8('a' = 1)</type>
+			</column>
+		</columns>
+	</meta>
+	<data>
+	</data>
+	<rows>0</rows>
+	<exception>Code: 36. : Unexpected value 99 in enum: While executing XMLRowOutputFormat. (BAD_ARGUMENTS) </exception>
+</result>
+With parallel formatting
+JSON
+1
+JSONCompact
+1
+JSONObjectEachRow
+1
+JSONEachRow
+1
+JSONCompactEachRow
+1
+Test 1
+1
+1
+Test 2
+1
+1
+Test 3
+1
+1

tests/queries/0_stateless/02841_valid_json_and_xml_on_http_exception.sh

@@ -4,7 +4,12 @@ CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
 # shellcheck source=../shell_config.sh
 . "$CUR_DIR"/../shell_config.sh
 
-CH_URL="$CLICKHOUSE_URL&http_write_exception_in_output_format=1&allow_experimental_analyzer=0"
+CH_URL_BASE="$CLICKHOUSE_URL&http_write_exception_in_output_format=1&allow_experimental_analyzer=0"
+
+for wait_end_of_query in 0 1
+do
+echo "wait_end_of_query=$wait_end_of_query"
+CH_URL="$CH_URL_BASE&wait_end_of_query=$wait_end_of_query"
 
 echo "One block"
 for parallel in 0 1
@@ -106,3 +111,4 @@ ${CLICKHOUSE_CURL} -sS "$CH_URL" -d "select * from test_02841 format JSON settin
 
 $CLICKHOUSE_CLIENT -q "drop table test_02841"
 
+done