Skip to content

Commit 32e7c3e

Browse files
authored
Add remote address information in SessionProtocolNegotiationException (#6113)
Motivation: Adding the remoteAddress (i.e., the actual host, IP and port) to the exception message will help users understand which specific endpoint was involved in the failed negotiation. ref: https://discord.com/channels/1087271586832318494/1087272728177942629/1341444142131183718 I appreciate any kind of feedback! Modifications: - Added SocketAddress remoteAddress in the exception message of SessionProtocolNegotiationException. Result: - Users will see the remoteAddress in the error message when a SessionProtocolNegotiationException occurs. - No behavioral changes, but the exception message format is slightly modified. <!-- Visit this URL to learn more about how to write a pull request description: https://armeria.dev/community/developer-guide#how-to-write-pull-request-description -->
1 parent 33219e0 commit 32e7c3e

File tree

4 files changed

+14
-6
lines changed

4 files changed

+14
-6
lines changed

core/src/main/java/com/linecorp/armeria/client/HttpChannelPool.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,9 @@ private void connect(SessionProtocol desiredProtocol, SerializationFormat serial
356356
notifyConnect(desiredProtocol, key,
357357
eventLoop.newFailedFuture(
358358
new SessionProtocolNegotiationException(
359-
desiredProtocol, "previously failed negotiation")),
359+
desiredProtocol,
360+
"previously failed negotiation (remoteAddress: " +
361+
remoteAddress + ')')),
360362
promise, timingsBuilder);
361363
return;
362364
}

core/src/main/java/com/linecorp/armeria/client/HttpClientPipelineConfigurator.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,8 @@ void finishWithNegotiationFailure(
461461
final ChannelPipeline pipeline = ctx.pipeline();
462462
pipeline.channel().eventLoop().execute(
463463
() -> pipeline.fireUserEventTriggered(
464-
new SessionProtocolNegotiationException(expected, actual, reason)));
464+
new SessionProtocolNegotiationException(
465+
expected, actual, reason + " (channel: " + pipeline.channel() + ')')));
465466
ctx.close();
466467
}
467468

core/src/main/java/com/linecorp/armeria/client/HttpSessionHandler.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ private void scheduleSessionTimeout(Channel channel, Promise<Channel> sessionPro
174174
sessionTimeoutFuture = channel.eventLoop().schedule(() -> {
175175
if (sessionPromise.tryFailure(new SessionProtocolNegotiationException(
176176
desiredProtocol,
177-
"connection established, but session creation timed out: " + channel))) {
177+
"connection established, but session creation timed out. (channel: " + channel + ')'))) {
178178
channel.close();
179179
}
180180
}, connectionTimeoutMillis, TimeUnit.MILLISECONDS);

core/src/test/java/com/linecorp/armeria/client/H2WithoutAlpnTest.java

+8-3
Original file line numberDiff line numberDiff line change
@@ -93,16 +93,21 @@ void shouldRejectH2WithoutAlpn() {
9393
.tlsCustomizer(b -> b.trustManager(cert.certificate()))
9494
.build()) {
9595

96-
final BlockingWebClient client = WebClient.builder("h2://127.0.0.1:" + server.httpPort())
96+
final String remote = "127.0.0.1";
97+
final BlockingWebClient client = WebClient.builder("h2://" + remote + ":" + server.httpPort())
9798
.factory(factory)
9899
.build()
99100
.blocking();
100101

101102
assertThatThrownBy(() -> client.get("/"))
102103
.isInstanceOf(UnprocessedRequestException.class)
103104
.hasCauseInstanceOf(SessionProtocolNegotiationException.class)
104-
.hasMessageContaining("expected: h2, actual: h1, " +
105-
"reason: unexpected protocol negotiation result");
105+
.hasMessageContainingAll(
106+
"expected: h2",
107+
"actual: h1",
108+
"reason: unexpected protocol negotiation result",
109+
remote
110+
);
106111
}
107112
}
108113
}

0 commit comments

Comments
 (0)