Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't log connection closed exceptions as ERROR in websockets-next #40246

Merged
merged 1 commit into from
May 9, 2024
Merged

Don't log connection closed exceptions as ERROR in websockets-next #40246

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 37 additions & 12 deletions ...s/websockets-next/runtime/src/main/java/io/quarkus/websockets/next/runtime/Endpoints.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ public void handle(Void event) {
LOG.debugf("@OnTextMessage callback consuming Multi completed: %s",
connection);
} else {
LOG.errorf(r.cause(),
"Unable to complete @OnTextMessage callback consuming Multi: %s",
logFailure(r.cause(),
"Unable to complete @OnTextMessage callback consuming Multi",
connection);
}
});
Expand All @@ -88,16 +88,16 @@ public void handle(Void event) {
LOG.debugf("@OnBinaryMessage callback consuming Multi completed: %s",
connection);
} else {
LOG.errorf(r.cause(),
"Unable to complete @OnBinaryMessage callback consuming Multi: %s",
logFailure(r.cause(),
"Unable to complete @OnBinaryMessage callback consuming Multi",
connection);
}
});
}
});
}
} else {
LOG.errorf(r.cause(), "Unable to complete @OnOpen callback: %s", connection);
logFailure(r.cause(), "Unable to complete @OnOpen callback", connection);
}
});
}
Expand All @@ -110,7 +110,7 @@ public void handle(Void event) {
if (r.succeeded()) {
LOG.debugf("@OnTextMessage callback consumed text message: %s", connection);
} else {
LOG.errorf(r.cause(), "Unable to consume text message in @OnTextMessage callback: %s",
logFailure(r.cause(), "Unable to consume text message in @OnTextMessage callback",
connection);
}
});
Expand All @@ -136,9 +136,9 @@ public void handle(Void event) {
binaryMessageHandler(connection, endpoint, ws, onOpenContext, m -> {
endpoint.onBinaryMessage(m).onComplete(r -> {
if (r.succeeded()) {
LOG.debugf("@OnBinaryMessage callback consumed text message: %s", connection);
LOG.debugf("@OnBinaryMessage callback consumed binary message: %s", connection);
} else {
LOG.errorf(r.cause(), "Unable to consume text message in @OnBinaryMessage callback: %s",
logFailure(r.cause(), "Unable to consume binary message in @OnBinaryMessage callback",
connection);
}
});
Expand All @@ -164,8 +164,7 @@ public void handle(Void event) {
if (r.succeeded()) {
LOG.debugf("@OnPongMessage callback consumed text message: %s", connection);
} else {
LOG.errorf(r.cause(), "Unable to consume text message in @OnPongMessage callback: %s",
connection);
logFailure(r.cause(), "Unable to consume text message in @OnPongMessage callback", connection);
}
});
});
Expand All @@ -192,7 +191,7 @@ public void handle(Void event) {
if (r.succeeded()) {
LOG.debugf("@OnClose callback completed: %s", connection);
} else {
LOG.errorf(r.cause(), "Unable to complete @OnClose callback: %s", connection);
logFailure(r.cause(), "Unable to complete @OnClose callback", connection);
}
onClose.run();
if (timerId != null) {
Expand All @@ -212,14 +211,40 @@ public void handle(Throwable t) {
public void handle(Void event) {
endpoint.doOnError(t).subscribe().with(
v -> LOG.debugf("Error [%s] processed: %s", t.getClass(), connection),
t -> LOG.errorf(t, "Unhandled error occured: %s", t.toString(),
t -> LOG.errorf(t, "Unhandled error occurred: %s", t.toString(),
connection));
}
});
}
});
}

private static void logFailure(Throwable throwable, String message, WebSocketConnectionBase connection) {
if (isWebSocketIsClosedFailure(throwable, connection)) {
LOG.debugf(throwable,
message + ": %s",
connection);
} else {
LOG.errorf(throwable,
message + ": %s",
connection);
}
}

private static boolean isWebSocketIsClosedFailure(Throwable throwable, WebSocketConnectionBase connection) {
if (!connection.isClosed()) {
return false;
}
if (throwable == null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a check that WebSocketConnectionBase#isClosed() returns true?

I wonder if we should create a Vertx issue to introduce ConnectionClosedException or something like that...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a check that WebSocketConnectionBase#isClosed() returns true?

Done

I wonder if we should create a Vertx issue to introduce ConnectionClosedException or something like that.

@vietj would you be willing to have that in Vertx?

return false;
}
String message = throwable.getMessage();
if (message == null) {
return false;
}
return message.contains("WebSocket is closed");
}

private static void textMessageHandler(WebSocketConnectionBase connection, WebSocketEndpoint endpoint, WebSocketBase ws,
Context context, Consumer<String> textAction, boolean newDuplicatedContext) {
ws.textMessageHandler(new Handler<String>() {
Expand Down