Skip to content

Commit dda2d3d

Browse files
schlosnapkoenig10
andauthored
Close exchange on closed client (#2523)
* Close exchange on closed client * Apply suggestions from code review Co-authored-by: Patrick Koenig <pkoenig10@gmail.com> --------- Co-authored-by: Patrick Koenig <pkoenig10@gmail.com>
1 parent 2081d70 commit dda2d3d

File tree

2 files changed

+27
-22
lines changed

2 files changed

+27
-22
lines changed

conjure-java-undertow-runtime/src/main/java/com/palantir/conjure/java/undertow/runtime/ConjureExceptions.java

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import java.io.OutputStream;
4242
import java.time.temporal.ChronoUnit;
4343
import java.util.Collections;
44+
import java.util.Objects;
4445
import java.util.Optional;
4546
import java.util.function.Consumer;
4647
import org.xnio.IoUtils;
@@ -57,6 +58,9 @@ public enum ConjureExceptions implements ExceptionHandler {
5758
private static final Serializer<ConjureError> serializer =
5859
new ConjureBodySerDe(Collections.singletonList(Encodings.json())).serializer(new TypeMarker<>() {});
5960

61+
private static final String COULD_NOT_READ_CONTENT_LENGTH_DATA_MESSAGE =
62+
UndertowMessages.MESSAGES.couldNotReadContentLengthData().getMessage();
63+
6064
// Log at most once every second
6165
private static final RateLimiter qosLoggingRateLimiter = RateLimiter.create(1);
6266

@@ -66,22 +70,22 @@ public void handle(HttpServerExchange exchange, Throwable throwable) {
6670
setFailure(exchange, throwable);
6771
if (throwable instanceof CheckedServiceException checkedServiceException) {
6872
checkedServiceException(exchange, checkedServiceException);
69-
} else if (throwable instanceof ServiceException) {
70-
serviceException(exchange, (ServiceException) throwable);
71-
} else if (throwable instanceof QosException) {
72-
qosException(exchange, (QosException) throwable);
73-
} else if (throwable instanceof RemoteException) {
74-
remoteException(exchange, (RemoteException) throwable);
73+
} else if (throwable instanceof ServiceException serviceException) {
74+
serviceException(exchange, serviceException);
75+
} else if (throwable instanceof QosException qosException) {
76+
qosException(exchange, qosException);
77+
} else if (throwable instanceof RemoteException remoteException) {
78+
remoteException(exchange, remoteException);
7579
} else if (throwable instanceof IllegalArgumentException) {
7680
illegalArgumentException(exchange, throwable);
77-
} else if (throwable instanceof FrameworkException) {
78-
frameworkException(exchange, (FrameworkException) throwable);
79-
} else if (throwable instanceof DeadlineExpiredException) {
80-
deadlineExpiredException(exchange, (DeadlineExpiredException) throwable);
81-
} else if (throwable instanceof Error) {
82-
error(exchange, (Error) throwable);
83-
} else if (throwable instanceof IOException) {
84-
ioException(exchange, (IOException) throwable);
81+
} else if (throwable instanceof FrameworkException frameworkException) {
82+
frameworkException(exchange, frameworkException);
83+
} else if (throwable instanceof DeadlineExpiredException deadlineExpiredException) {
84+
deadlineExpiredException(exchange, deadlineExpiredException);
85+
} else if (throwable instanceof Error error) {
86+
error(exchange, error);
87+
} else if (throwable instanceof IOException ioException) {
88+
ioException(exchange, ioException);
8589
} else {
8690
ServiceException exception = new ServiceException(ErrorType.INTERNAL, throwable);
8791
log(exception, throwable);
@@ -206,15 +210,12 @@ private static void ioException(HttpServerExchange exchange, IOException ioExcep
206210
return;
207211
}
208212

209-
if (ioException
210-
.getMessage()
211-
.equals(UndertowMessages.MESSAGES
212-
.couldNotReadContentLengthData()
213-
.getMessage())) {
213+
if (Objects.equals(ioException.getMessage(), COULD_NOT_READ_CONTENT_LENGTH_DATA_MESSAGE)) {
214214
log.info(
215-
"Remote peer closed connection before all data could be read. The request may have been aborted by"
216-
+ " the client",
215+
"Remote peer closed connection before all data could be read. "
216+
+ "The request may have been aborted by the client.",
217217
ioException);
218+
IoUtils.safeClose(exchange.getConnection());
218219
return;
219220
}
220221

conjure-java-undertow-runtime/src/test/java/com/palantir/conjure/java/undertow/runtime/ConjureExceptionHandlerTest.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import static org.assertj.core.api.Assertions.assertThat;
2020
import static org.assertj.core.api.Assertions.assertThatCode;
21+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2122
import static org.assertj.core.api.InstanceOfAssertFactories.LIST;
2223
import static org.assertj.core.api.InstanceOfAssertFactories.MAP;
2324

@@ -46,6 +47,7 @@
4647
import java.io.IOException;
4748
import java.io.InputStream;
4849
import java.net.HttpURLConnection;
50+
import java.net.SocketException;
4951
import java.net.URL;
5052
import java.nio.charset.StandardCharsets;
5153
import java.time.Duration;
@@ -366,7 +368,9 @@ public void handlesErrorWithoutRethrowing() {
366368
public void handlesPeerRemoteExceptionsWithoutRethrowing() throws IOException {
367369
exception = UndertowMessages.MESSAGES.couldNotReadContentLengthData();
368370
HttpURLConnection connection = execute();
369-
assertThat(connection.getResponseCode()).isEqualTo(200);
371+
assertThatThrownBy(() -> connection.getResponseCode())
372+
.isInstanceOf(SocketException.class)
373+
.hasMessageContaining("Unexpected end of file from server");
370374
}
371375

372376
private static String getErrorBody(HttpURLConnection connection) {

0 commit comments

Comments
 (0)