From d2d5fb6e95e7c08db6d9d68ae168698d96a17807 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Thu, 11 Feb 2021 11:36:23 +0800 Subject: [PATCH] [C++] Add detail logs for schema related messages (#9544) ### Motivation If broker failed to parse the schema, the logs of C++/Python client is poor because when C++ client handles the `Error` response, it doesn't print the `message` field that is filled by broker. On the other hand, if the `InvalidSchemaDataException` is thrown by `StructSchemaDataValidator#throwInvalidSchemaDataException`, the detail error message will be written to the exception's cause. The String that `getMessage()` returns is not enough ### Modifications - When `tryAddSchema` failed, add the exception cause's message to the error message that's sent to client. - Print `Error` response's `message` field to logs. ### Verifying this change - [ ] Make sure that the change passes the CI checks. This change is a trivial rework / code cleanup without any test coverage. Take #9483 (which is to be fixed) as the example, before this PR, the client side's log is like > Received error response from server: IncompatibleSchema -- req_id: 0 After this PR, the client side's log will be > Received error response from server: IncompatibleSchema (Invalid schema definition data for AVRO schema caused by org.apache.avro.SchemaParseException: Undefined name: "array") -- req_id: 0 --- .../org/apache/pulsar/broker/service/ServerCnx.java | 12 ++++++++++-- pulsar-client-cpp/lib/ClientConnection.cc | 1 + 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java index 15cb400dccbf0..5df95ff747204 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java @@ -1167,9 +1167,13 @@ protected void handleProducer(final CommandProducer cmdProducer) { CompletableFuture schemaVersionFuture = tryAddSchema(topic, schema); schemaVersionFuture.exceptionally(exception -> { + String message = exception.getMessage(); + if (exception.getCause() != null) { + message += (" caused by " + exception.getCause()); + } commandSender.sendErrorResponse(requestId, BrokerServiceException.getClientErrorCode(exception), - exception.getMessage()); + message); producers.remove(producerId, producerFuture); return null; }); @@ -1727,7 +1731,11 @@ protected void handleGetOrCreateSchema(CommandGetOrCreateSchema commandGetOrCrea CompletableFuture schemaVersionFuture = tryAddSchema(topic, schema); schemaVersionFuture.exceptionally(ex -> { ServerError errorCode = BrokerServiceException.getClientErrorCode(ex); - commandSender.sendGetOrCreateSchemaErrorResponse(requestId, errorCode, ex.getMessage()); + String message = ex.getMessage(); + if (ex.getCause() != null) { + message += (" caused by " + ex.getCause()); + } + commandSender.sendGetOrCreateSchemaErrorResponse(requestId, errorCode, message); return null; }).thenAccept(schemaVersion -> { commandSender.sendGetOrCreateSchemaResponse(requestId, schemaVersion); diff --git a/pulsar-client-cpp/lib/ClientConnection.cc b/pulsar-client-cpp/lib/ClientConnection.cc index 69f889c3ef1d9..28a306c2baee0 100644 --- a/pulsar-client-cpp/lib/ClientConnection.cc +++ b/pulsar-client-cpp/lib/ClientConnection.cc @@ -976,6 +976,7 @@ void ClientConnection::handleIncomingCommand() { const CommandError& error = incomingCmd_.error(); Result result = getResult(error.error()); LOG_WARN(cnxString_ << "Received error response from server: " << result + << (error.has_message() ? (" (" + error.message() + ")") : "") << " -- req_id: " << error.request_id()); Lock lock(mutex_);