Fix wrong exception propagation on connection errors and channel binding failure#1676
Fix wrong exception propagation on connection errors and channel binding failure#1676tsegismont wants to merge 1 commit into
Conversation
|
Hi @tsegismont I tested your branch but I don't get the real message, it seems that it still returns the ClosedConnectionException instead of the actual error. Could you please confirm what I'm doing wrong? Vertx vertx = Vertx.vertx();
PgConnectOptions connectOptions = new PgConnectOptions()
.setPort(5432)
.setHost("localhost")
.setDatabase("postgres")
.setUser("postgres")
.setSslOptions(new ClientSSLOptions().setTrustAll(true))
.setSslMode(SslMode.DISABLE)
.setChannelBinding(io.vertx.pgclient.ChannelBinding.REQUIRE)
.setPassword("123");
PgConnection.connect(vertx, connectOptions)
.onComplete(res -> {
if (res.succeeded()) {
System.out.println("Connected...");
} else {
System.out.println("Could not connect: " + res.cause().getMessage());
}
});
vertx.close(); |
|
my branch does not aim to fix the bug we are currently observing in this issue, it aims first to get the exception handling flow correct from the NetSocket / SocketConnectionBase perspective before doing any other fix. |
|
in particular : handleException should not close the socket in your patch because the socket will be closed by the code calling handleException, this is what I'd like to clarify and avoid. |
f3bf835 to
df2bd76
Compare
|
@vietj please take another look I had to make a special case in
Nevertheless, I don't believe we have a way to close the connection abruptly like PostgreSQL expects:
https://www.postgresql.org/docs/current/protocol-flow.html I believe this would require changes in |
…ing failure See eclipse-vertx#1672, eclipse-vertx#1674, eclipse-vertx#1675 Inflight commands should not fail with ClosedConnectionException but with the actual error, e.g. a PgException reporting SQLSTATE 57P01 when pg_terminate_backend kills a connection mid-flight. When channel binding is required but SSL is not available, the PostgreSQL protocol recommends immediately closing the connection. The ChannelBindingException is now correctly reported to the caller rather than being masked by a subsequent ClosedConnectionException. Some portions of this content were created with the assistance of Claude Code. Signed-off-by: Thomas Segismont <tsegismont@gmail.com> Fixup Signed-off-by: Thomas Segismont <tsegismont@gmail.com>
df2bd76 to
c4a36e4
Compare
|
Well in fact, |
|
your changes in |
|
another way would be to close the ChannelHandlerContext ? |
See #1672, #1674, #1675
When a connection handles a failure, the underlying Netty socket should be closed, avoiding leak of file descriptors and Netty resources.
Additionally, inflight commands shouldn't fail with ClosedConnectionException but with the actual error.
Some portions of this content were created with the assistance of Claude Code.