Skip to content

Commit fc4a61a

Browse files
committed
Clarify SocketConnectionBase exception handling flow report.
Motivation: SocketConnectionBase handleException method should only be called by the underlying socket exception handler, when it happens the close handler will be called subsequently by the socket. Changes: Instead of calling the internal handleClose from the handleException method, we should capture the exception reported and reuse it when handleClosed is called. Remove the handleException call from the try/catch block in handleMessage, handleMessage should never throw an exception, if it does, it will be logged by the socket calling handleMessage (that would be a bug). Remove the PgSocketConnection handleException override that reports the exception, since now the base method will capture the exception, the handleClose will call reportException instead. Remove unused DB2SocketConnection code that was doing nothing. Make a few methods private to SocketConnectionBase to ensure they are only called in an expected way.
1 parent 5869727 commit fc4a61a

4 files changed

Lines changed: 12 additions & 34 deletions

File tree

vertx-db2-client/src/main/java/io/vertx/db2client/impl/DB2SocketConnection.java

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ public class DB2SocketConnection extends SocketConnectionBase {
4343

4444
private final DB2ConnectOptions connectOptions;
4545
private DB2Codec codec;
46-
private Handler<Void> closeHandler;
4746
public final ConnectionMetaData connMetadata = new ConnectionMetaData();
4847
public ConnectionState status = ConnectionState.CONNECTING;
4948

@@ -128,14 +127,6 @@ protected <R> void doSchedule(CommandBase<R> cmd, Completable<R> handler) {
128127
}
129128
}
130129

131-
@Override
132-
public void handleClose(Throwable t) {
133-
super.handleClose(t);
134-
if (closeHandler != null) {
135-
context().runOnContext(closeHandler);
136-
}
137-
}
138-
139130
@Override
140131
public String system() {
141132
return "db2";
@@ -146,8 +137,4 @@ public DatabaseMetadata databaseMetadata() {
146137
return connMetadata.getDbMetadata();
147138
}
148139

149-
public DB2SocketConnection closeHandler(Handler<Void> handler) {
150-
closeHandler = handler;
151-
return this;
152-
}
153140
}

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

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -121,15 +121,6 @@ protected void handleMessage(Object msg) {
121121
}
122122
}
123123

124-
@Override
125-
protected void handleException(Throwable t) {
126-
if (t instanceof PgException) {
127-
reportException(t);
128-
} else {
129-
super.handleException(t);
130-
}
131-
}
132-
133124
public int getProcessId() {
134125
return processId;
135126
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,11 @@ public void testCancelRequest(TestContext ctx) {
134134

135135
@Test
136136
public void testInflightCommandsFailWhenConnectionClosed(TestContext ctx) {
137+
Async async = ctx.async();
137138
connector.accept(ctx.asyncAssertSuccess(conn1 -> {
139+
conn1.exceptionHandler(err -> {
140+
async.complete();
141+
});
138142
conn1
139143
.query("SELECT pg_backend_pid()")
140144
.execute()

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

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ public enum Status {
8787
protected final NetSocketInternal socket;
8888
protected Status status = Status.CONNECTED;
8989

90+
private Throwable exceptionReport;
91+
9092
public SocketConnectionBase(NetSocketInternal socket,
9193
ClientMetrics metrics,
9294
boolean cachePreparedStatements,
@@ -147,13 +149,7 @@ public io.vertx.core.Context context() {
147149
public void init() {
148150
socket.closeHandler(this::handleClosed);
149151
socket.exceptionHandler(this::handleException);
150-
socket.messageHandler(msg -> {
151-
try {
152-
handleMessage(msg);
153-
} catch (Exception e) {
154-
handleException(e);
155-
}
156-
});
152+
socket.messageHandler(this::handleMessage);
157153
socket.readCompletionHandler(this::handleReadComplete);
158154
}
159155

@@ -413,26 +409,26 @@ private void removeCachedStatement(String sql) {
413409
}
414410

415411
private void handleClosed(Void v) {
416-
handleClose(null);
412+
handleClose(exceptionReport);
417413
}
418414

419-
protected void handleException(Throwable t) {
415+
private void handleException(Throwable t) {
420416
if (t instanceof DecoderException) {
421417
DecoderException err = (DecoderException) t;
422418
t = err.getCause();
423419
}
424-
handleClose(t);
420+
exceptionReport = t;
425421
}
426422

427-
protected void reportException(Throwable t) {
423+
private void reportException(Throwable t) {
428424
synchronized (this) {
429425
if (holder != null) {
430426
holder.handleException(t);
431427
}
432428
}
433429
}
434430

435-
protected void handleClose(Throwable t) {
431+
private void handleClose(Throwable t) {
436432
if (status != Status.CLOSED) {
437433
status = Status.CLOSED;
438434
if (metrics != null) {

0 commit comments

Comments
 (0)