Add more info in the error messages

[zymap]

---

Modifications

Add topic and subscription info in the error message to help locate the problem that which topic or subscription throws the error.

[codelipenghui]

run java8 tests
run integration tests

[sijie]

There are multiple places that TimeoutException can occur. I would suggest fixing all places that TimeoutException is thrown.

• ClientCnx: topic lookup requests can be timed out.
• Producer: send requests can be timed out.
• Consumer: getLastMessageId can be timed out.

I am sure there are more client exceptions that require topic name in the error messages. so please fix them as well.

[sijie]

run java8 tests

[zymap]

The unit test seems failed caused by my fix. I am fixing the test.

[zymap]

@sijie
There are two ways when we threw an Exception.

One is we get an exception and throw it out.

future.exceptionally(e -> {
       anotherFuture.completeExceptionally(e)
})

Another is we wrap the exception with the PulsarClientException.

future.exceptionally(e -> {
       anotherFuture.completeExceptinoally(new PulsarClientException())
})

If the exception belongs to the second one, I can add the info message easily. Otherwise, I think it has a risk to update the Exception If I don't know the exception is caught by others for retrying or do something others.

How do you think?

[sijie]

In the first case, you need to find where the exception was first raised and fix it, no?

[zymap]

Yes. There are many exceptions thrown by ClientCnx. In ClientCnx, it using a map to save requestId and CompleableFuture. And the requestId is generated from other places like ConsumerImpl or ProducerImpl or LookupService. Should I pass the topic name from the Consumerimpl and combine it with requestId? If so I can get the topic name or other information when the future complete exceptionally.

[sijie]

for the exceptions raised by ClientCnx, it is making more sense to catch the exception at the caller (Producer, Consumer) and attach the topic information to the exception, rather than passing the topic name information into ClientCnx.

[sijie]

I am not sure WrapperException is the right fix here. because it will change the client behavior. for example, if a client used to handle exeception A, now we are throwing a WrapperException of A.

We should reconstruct a A with an updated error message and its stack trace.

[zymap]

if the A is thrown by ClientCnx, we reconstruct the A it might still change the client's behavior.

[zymap]

But it seems if I want to add the error message there are two ways to update. One is updating the error message before the exception throws. Another is unwrapping the wrapper when the method needs to handle the exception.

[zymap]

@sijie I add a new method wrap to wrap the exception to enriching the exception message info.

The wrap method will get the exception and wrap the old exception with the same type of the exception, so I can add more message info to the exception. I run the test that failed before, it can be passed on my laptop.

What do you think?

[zymap]

run java8 tests
run integration tests
run cpp tests

[zymap]

@sijie PTAL. Thanks

[sijie]

run cpp tests

[zymap]

run cpp tests

[zymap]

@sijie PTAL. Thanks.