Skip to content

Commit f07e4fb

Browse files
committed
fix(tracing): stop 'Unmatched response message' logs for non-stream IO
- Previously only StreamMessageConsumer was treated as "sending". With WebSocket and other non-stream transports, sentRequests stayed empty, so every incoming response looked unmatched, emitted a warning, and produced no trace. - Now any consumer that is not a RemoteEndpoint is treated as "sending", ensuring sentRequests is populated regardless of transport and responses are matched correctly. - Removed the "Unknown MessageConsumer type" branch to avoid misclassification and improve compatibility with custom/non-stream transports.
1 parent 79e2cf7 commit f07e4fb

File tree

2 files changed

+71
-7
lines changed

2 files changed

+71
-7
lines changed

org.eclipse.lsp4j.jsonrpc/src/main/java/org/eclipse/lsp4j/jsonrpc/TracingMessageConsumer.java

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
package org.eclipse.lsp4j.jsonrpc;
1313

1414
import org.eclipse.lsp4j.jsonrpc.json.MessageJsonHandler;
15-
import org.eclipse.lsp4j.jsonrpc.json.StreamMessageConsumer;
1615
import org.eclipse.lsp4j.jsonrpc.messages.Message;
1716
import org.eclipse.lsp4j.jsonrpc.messages.NotificationMessage;
1817
import org.eclipse.lsp4j.jsonrpc.messages.RequestMessage;
@@ -102,13 +101,12 @@ public void consume(Message message) throws MessageIssueException, JsonRpcExcept
102101
final String date = dateTimeFormatter.format(now);
103102
final String logString;
104103

105-
if (messageConsumer instanceof StreamMessageConsumer) {
106-
logString = consumeMessageSending(message, now, date);
107-
} else if (messageConsumer instanceof RemoteEndpoint) {
104+
if (messageConsumer instanceof RemoteEndpoint) {
108105
logString = consumeMessageReceiving(message, now, date);
109106
} else {
110-
LOG.log(WARNING, String.format("Unknown MessageConsumer type: %s", messageConsumer));
111-
logString = null;
107+
// Treat any non-RemoteEndpoint consumer as an outgoing transport (sending),
108+
// so tracing works for WebSocket and other non-stream consumers as well.
109+
logString = consumeMessageSending(message, now, date);
112110
}
113111

114112
if (logString != null) {

org.eclipse.lsp4j.jsonrpc/src/test/java/org/eclipse/lsp4j/jsonrpc/test/TracingMessageConsumerTest.java

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
import java.util.concurrent.CompletableFuture;
3434

3535
import static java.util.Collections.emptyMap;
36-
import static org.junit.Assert.assertEquals;
36+
import static org.junit.Assert.*;
3737

3838
public class TracingMessageConsumerTest {
3939
private static final RemoteEndpoint TEST_REMOTE_ENDPOINT = new EmptyRemoteEndpoint();
@@ -299,6 +299,72 @@ public void testSendingNotificationWithCustomAdapter() {
299299

300300
assertEquals(expectedTrace, actualTrace);
301301
}
302+
303+
@Test
304+
public void testUnmatchedResponseWhenOutgoingIsNotStreamConsumer() {
305+
// Simulate an outgoing transport that is not StreamMessageConsumer (e.g., websocket)
306+
var outgoingTrace = new StringWriter();
307+
var outgoingPrintWriter = new PrintWriter(outgoingTrace);
308+
309+
var incomingTrace = new StringWriter();
310+
var incomingPrintWriter = new PrintWriter(incomingTrace);
311+
312+
var sentRequests = new HashMap<String, RequestMetadata>();
313+
var receivedRequests = new HashMap<String, RequestMetadata>();
314+
315+
// Wrap a non-stream consumer so TracingMessageConsumer does not treat it as "sending"
316+
var outgoingTracing = new TracingMessageConsumer(new EmptyMessageConsumer(), sentRequests, receivedRequests, outgoingPrintWriter, TEST_CLOCK_1, Locale.US);
317+
318+
// Send a request (this will NOT populate sentRequests because consumer is not StreamMessageConsumer)
319+
var req = new RequestMessage();
320+
req.setId("1");
321+
req.setMethod("foo");
322+
outgoingTracing.consume(req);
323+
324+
// Prepare to capture warnings from TracingMessageConsumer
325+
var logger = java.util.logging.Logger.getLogger(TracingMessageConsumer.class.getName());
326+
var warnings = new java.util.ArrayList<String>();
327+
var handler = new java.util.logging.Handler() {
328+
@Override
329+
public void publish(java.util.logging.LogRecord record) {
330+
if (record.getLevel() == java.util.logging.Level.WARNING)
331+
warnings.add(record.getMessage());
332+
}
333+
334+
@Override
335+
public void flush() {
336+
}
337+
338+
@Override
339+
public void close() throws SecurityException {
340+
}
341+
};
342+
logger.addHandler(handler);
343+
try {
344+
// Now receive a response on the RemoteEndpoint path. Expected behavior (desired):
345+
// no warnings and a proper response trace even when the outgoing transport
346+
// is not a StreamMessageConsumer (e.g., websockets).
347+
var incomingTracing = new TracingMessageConsumer(TEST_REMOTE_ENDPOINT, sentRequests, receivedRequests, incomingPrintWriter, TEST_CLOCK_2, Locale.US);
348+
349+
var resp = new ResponseMessage();
350+
resp.setId("1");
351+
resp.setResult("ok");
352+
incomingTracing.consume(resp);
353+
354+
// Assert NO warning and a proper trace line for the response
355+
boolean sawUnmatched = warnings.stream().anyMatch(m -> m.contains("Unmatched response message"));
356+
assertFalse("Did not expect unmatched response warning", sawUnmatched);
357+
String expectedTrace = "" +
358+
"[Trace - 06:07:30 PM] Received response 'foo - (1)' in 100ms\n" +
359+
"Result: \"ok\"\n" +
360+
"Error: null\n" +
361+
"\n" +
362+
"\n";
363+
assertEquals(expectedTrace, incomingTrace.toString());
364+
} finally {
365+
logger.removeHandler(handler);
366+
}
367+
}
302368
}
303369

304370
class EmptyRemoteEndpoint extends RemoteEndpoint {

0 commit comments

Comments
 (0)