[Bug] ControlledClusterFailover.close loses the underlying exception

[PavelZeger]

Search before reporting

• I searched in the issues and found nothing similar.

Read release policy

• I understand that unsupported versions don't get bug fixes. I will attempt to reproduce the issue on a supported version of Pulsar client and Pulsar broker.

User environment

Client Java

Issue Description

pulsar-client/src/main/java/org/apache/pulsar/client/impl/ControlledClusterFailover.java:

@Override
public void close() {
    this.executor.shutdown();
    if (httpClient != null) {
        try {
            httpClient.close();
        } catch (IOException e) {
            log.error("Failed to close http client.");
        }
    }
}

The IOException e is captured but never logged.

When the HTTP client fails to close (which happens on socket leaks, half-closed connections, or shutdown races), the operator sees one line:

ERROR ...ControlledClusterFailover - Failed to close http client.

and no information about what went wrong. The IOException could be:

• SocketTimeoutException - the connection didn't drain in time.
• IOException("Connection reset") - the proxy dropped us.
• IOException("Idle eviction failed") - an internal Jetty error.

All three suggest different remedies, and the operator can't tell which without enabling DEBUG logs.
When a client transitions away from a cluster, this close() runs. Diagnostic information here is the only signal an operator has that the failover happened cleanly.

Error messages

Reproducing the issue

N/A

Additional information

No response

Are you willing to submit a PR?

• I'm willing to submit a PR!

[PavelZeger]

I would like to create PR for this bug

[lhotari]

When the HTTP client fails to close (which happens on socket leaks, half-closed connections, or shutdown races)

This doesn't seem to be a realistic analysis of when it could fail to close. Most likely the exception is never thrown and the reason it needs to be caught is that it's a checked exception. However, I do agree that swallowing exceptions isn't a good practice.

[PavelZeger]

When the HTTP client fails to close (which happens on socket leaks, half-closed connections, or shutdown races)

This doesn't seem to be a realistic analysis of when it could fail to close. Most likely the exception is never thrown and the reason it needs to be caught is that it's a checked exception. However, I do agree that swallowing exceptions isn't a good practice.

I didn't want to leave it as is with swallowing exception thus I've preferred to open a PR and fix it.