Skip to content

Commit c5794a6

Browse files
authored
Fix wrong exception propagation on connection errors and channel binding failure (#1676) (#1679)
See #1672, #1674, #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. Fixup Signed-off-by: Thomas Segismont <tsegismont@gmail.com>
1 parent 261ca55 commit c5794a6

4 files changed

Lines changed: 50 additions & 22 deletions

File tree

vertx-mssql-client/src/test/java/io/vertx/tests/mssqlclient/MSSQLQueriesTest.java

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,7 @@
4040
import java.util.stream.Collectors;
4141
import java.util.stream.Stream;
4242

43-
import static org.hamcrest.CoreMatchers.instanceOf;
44-
import static org.hamcrest.CoreMatchers.is;
43+
import static org.hamcrest.CoreMatchers.*;
4544
import static org.hamcrest.MatcherAssert.assertThat;
4645

4746
@RunWith(VertxUnitRunner.class)
@@ -206,4 +205,22 @@ public void testQuerySequences(TestContext ctx) {
206205
}));
207206
}));
208207
}
208+
209+
@Test
210+
public void testUnsupportedXmlTypeErrorPropagation(TestContext ctx) {
211+
connection
212+
.query("CREATE TABLE #TempXmlTest (id INT, xmlData XML)")
213+
.execute()
214+
.onComplete(ctx.asyncAssertSuccess(create -> {
215+
connection
216+
.query("SELECT * FROM #TempXmlTest")
217+
.execute()
218+
.onComplete(ctx.asyncAssertFailure(t -> {
219+
ctx.verify(v -> {
220+
assertThat(t, instanceOf(UnsupportedOperationException.class));
221+
assertThat(t.getMessage(), containsString("XML"));
222+
});
223+
}));
224+
}));
225+
}
209226
}

vertx-pg-client/src/main/java/io/vertx/pgclient/impl/codec/InitPgCommandMessage.java

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,19 @@
1616
*/
1717
package io.vertx.pgclient.impl.codec;
1818

19-
import java.nio.charset.Charset;
20-
import java.nio.charset.StandardCharsets;
21-
2219
import io.netty.buffer.ByteBuf;
2320
import io.vertx.core.VertxException;
2421
import io.vertx.pgclient.impl.PgDatabaseMetadata;
2522
import io.vertx.pgclient.impl.PgSocketConnection;
2623
import io.vertx.pgclient.impl.auth.scram.ScramAuthentication;
2724
import io.vertx.pgclient.impl.auth.scram.ScramSession;
28-
import io.vertx.sqlclient.spi.connection.Connection;
2925
import io.vertx.sqlclient.codec.CommandResponse;
26+
import io.vertx.sqlclient.spi.connection.Connection;
3027
import io.vertx.sqlclient.spi.protocol.InitCommand;
3128

29+
import java.nio.charset.Charset;
30+
import java.nio.charset.StandardCharsets;
31+
3232
class InitPgCommandMessage extends PgCommandMessage<Connection, InitCommand> {
3333

3434
private PgEncoder encoder;
@@ -65,9 +65,17 @@ void handleAuthenticationSasl(ByteBuf in) {
6565
throw new VertxException("Scram authentication not supported, missing com.ongres.scram:scram-client on the class/module path");
6666
}
6767
scramSession = scramAuth.session(cmd.username(), cmd.password().toCharArray());
68-
encoder.writeScramClientInitialMessage(
68+
try {
69+
encoder.writeScramClientInitialMessage(
6970
scramSession.createInitialSaslMessage(in, encoder.channelHandlerContext()));
70-
encoder.flush();
71+
encoder.flush();
72+
} catch (RuntimeException e) {
73+
decoder.fireCommandResponse(CommandResponse.failure(e));
74+
// If the frontend does not support the authentication method requested by the server,
75+
// then it should immediately close the connection.
76+
// See https://www.postgresql.org/docs/current/protocol-flow.html
77+
encoder.close();
78+
}
7179
}
7280

7381
@Override

vertx-pg-client/src/test/java/io/vertx/tests/pgclient/PgConnectionTest.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
import io.vertx.ext.unit.Async;
2121
import io.vertx.ext.unit.TestContext;
2222
import io.vertx.pgclient.PgConnection;
23-
import io.vertx.sqlclient.ClosedConnectionException;
23+
import io.vertx.pgclient.PgException;
2424
import io.vertx.sqlclient.Row;
2525
import io.vertx.sqlclient.RowSet;
2626
import io.vertx.sqlclient.Tuple;
@@ -148,7 +148,12 @@ public void testInflightCommandsFailWhenConnectionClosed(TestContext ctx) {
148148
.query("SELECT pg_sleep(20)")
149149
.execute()
150150
.onComplete(ctx.asyncAssertFailure(t -> {
151-
ctx.assertTrue(t instanceof ClosedConnectionException);
151+
if (t instanceof PgException) {
152+
PgException pge = (PgException) t;
153+
ctx.assertEquals("57P01", pge.getSqlState()); // ADMIN SHUTDOWN
154+
} else {
155+
ctx.fail(t);
156+
}
152157
}));
153158
connector.accept(ctx.asyncAssertSuccess(conn2 -> {
154159
conn2

vertx-sql-client-codec/src/main/java/io/vertx/sqlclient/codec/SocketConnectionBase.java

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,28 +21,25 @@
2121
import io.netty.channel.ChannelFutureListener;
2222
import io.netty.channel.ChannelHandlerContext;
2323
import io.netty.handler.codec.DecoderException;
24-
import io.vertx.core.*;
24+
import io.vertx.core.Completable;
25+
import io.vertx.core.Vertx;
26+
import io.vertx.core.VertxException;
27+
import io.vertx.core.internal.ContextInternal;
2528
import io.vertx.core.internal.logging.Logger;
2629
import io.vertx.core.internal.logging.LoggerFactory;
30+
import io.vertx.core.internal.net.NetSocketInternal;
2731
import io.vertx.core.net.SocketAddress;
2832
import io.vertx.core.spi.metrics.ClientMetrics;
2933
import io.vertx.core.tracing.TracingPolicy;
30-
import io.vertx.core.internal.ContextInternal;
31-
import io.vertx.core.internal.net.NetSocketInternal;
3234
import io.vertx.sqlclient.ClosedConnectionException;
3335
import io.vertx.sqlclient.Row;
3436
import io.vertx.sqlclient.SqlConnectOptions;
3537
import io.vertx.sqlclient.codec.impl.PreparedStatementCache;
36-
import io.vertx.sqlclient.spi.connection.Connection;
3738
import io.vertx.sqlclient.internal.PreparedStatement;
3839
import io.vertx.sqlclient.spi.DatabaseMetadata;
40+
import io.vertx.sqlclient.spi.connection.Connection;
3941
import io.vertx.sqlclient.spi.connection.ConnectionContext;
40-
import io.vertx.sqlclient.spi.protocol.CloseConnectionCommand;
41-
import io.vertx.sqlclient.spi.protocol.CloseStatementCommand;
42-
import io.vertx.sqlclient.spi.protocol.CommandBase;
43-
import io.vertx.sqlclient.spi.protocol.CompositeCommand;
44-
import io.vertx.sqlclient.spi.protocol.ExtendedQueryCommand;
45-
import io.vertx.sqlclient.spi.protocol.PrepareStatementCommand;
42+
import io.vertx.sqlclient.spi.protocol.*;
4643

4744
import java.util.ArrayDeque;
4845
import java.util.List;
@@ -437,11 +434,12 @@ private void handleClose(Throwable t) {
437434
if (t != null) {
438435
reportException(t);
439436
}
437+
Throwable inflightCause = t != null ? t : ClosedConnectionException.INSTANCE;
440438
CommandMessage<?, ?> msg;
441439
while ((msg = inflights.poll()) != null) {
442-
fail(msg, ClosedConnectionException.INSTANCE);
440+
fail(msg, inflightCause);
443441
}
444-
Throwable cause = t == null ? VertxException.noStackTrace(PENDING_CMD_CONNECTION_CORRUPT_MSG) : new VertxException(PENDING_CMD_CONNECTION_CORRUPT_MSG, t);
442+
Throwable cause = t == null ? VertxException.noStackTrace(PENDING_CMD_CONNECTION_CORRUPT_MSG) : VertxException.noStackTrace(PENDING_CMD_CONNECTION_CORRUPT_MSG, t);
445443
CommandBase<?> cmd;
446444
while ((cmd = pending.poll()) != null) {
447445
CommandBase c = cmd;

0 commit comments

Comments
 (0)