Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -647,8 +647,12 @@ private void verifyProtocolVersion(Connection connection) throws Exception {
if (this.options.getOnGitHubTelemetry() != null) {
connectParams.put("enableGitHubTelemetryForwarding", true);
}
var connectResponse = connection.rpc.invoke("connect", connectParams, ConnectResult.class).get(30,
TimeUnit.SECONDS);
// A legacy server rejects 'connect' and we fall back to 'ping' below, so
// only that rejection is expected; anything else stays a warning.
var connectResponse = connection.rpc
.invoke("connect", connectParams, ConnectResult.class,
cause -> cause instanceof JsonRpcException rpcEx && isUnsupportedConnectMethod(rpcEx))
.get(30, TimeUnit.SECONDS);
serverVersion = connectResponse.protocolVersion() != null
? connectResponse.protocolVersion().intValue()
: null;
Expand Down
28 changes: 27 additions & 1 deletion java/sdk/src/main/java/com/github/copilot/JsonRpcClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.util.concurrent.atomic.AtomicLong;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
import java.util.function.Predicate;
import java.util.logging.Level;
import java.util.logging.Logger;

Expand Down Expand Up @@ -134,6 +135,22 @@ public void registerMethodHandler(String method, BiConsumer<String, JsonNode> ha
* Sends a JSON-RPC request and waits for the response.
*/
public <T> CompletableFuture<T> invoke(String method, Object params, Class<T> responseType) {
return invoke(method, params, responseType, ex -> false);
}

/**
* Sends a JSON-RPC request and waits for the response, logging the failure at
* {@link Level#FINE} when {@code expectedFailure} accepts it.
*
* <p>
* Callers that recover from one specific failure (for example probing for a
* method that older servers do not implement) use this so the recovered failure
* is not surfaced to users as a warning with a stack trace. The predicate
* receives the unwrapped cause; every failure it rejects is still logged at
* {@link Level#WARNING}.
*/
<T> CompletableFuture<T> invoke(String method, Object params, Class<T> responseType,
Predicate<Throwable> expectedFailure) {
long timingNanos = System.nanoTime();
long id = requestIdCounter.incrementAndGet();
var future = new CompletableFuture<JsonNode>();
Expand Down Expand Up @@ -167,14 +184,23 @@ public <T> CompletableFuture<T> invoke(String method, Object params, Class<T> re
throw new CompletionException(e);
}
}).exceptionally(ex -> {
LoggingHelpers.logTiming(LOG, Level.WARNING, ex,
Level failureLevel = expectedFailure.test(unwrapCompletion(ex)) ? Level.FINE : Level.WARNING;
LoggingHelpers.logTiming(LOG, failureLevel, ex,
"JsonRpc.invoke JSON-RPC request finished. Elapsed={Elapsed}, Method=" + method + ", RequestId="
+ id + ", Status=Failed",
timingNanos);
throw ex instanceof RuntimeException re ? re : new RuntimeException(ex);
});
}

private static Throwable unwrapCompletion(Throwable ex) {
Throwable cause = ex;
while (cause instanceof CompletionException && cause.getCause() != null) {
cause = cause.getCause();
}
return cause;
}

/**
* Sends a JSON-RPC notification (no response expected).
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,18 @@
import java.net.ServerSocket;
import java.net.Socket;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Function;
import java.util.logging.Handler;
import java.util.logging.Level;
import java.util.logging.LogRecord;
import java.util.logging.Logger;

import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -456,4 +463,81 @@ void testCloseWithPendingRequests() throws Exception {
pair.serverSide.close();
pair.serverSocket.close();
}

// ---- invoke() failure log level ----

private static final class RecordingLogHandler extends Handler {

private final List<LogRecord> records = new CopyOnWriteArrayList<>();

@Override
public void publish(LogRecord record) {
records.add(record);
}

@Override
public void flush() {
}

@Override
public void close() {
}
}

/**
* Runs an invocation that the server rejects with {@code errorCode} and returns
* everything the {@link JsonRpcClient} logger emitted while it ran.
*/
private List<LogRecord> captureLogsForFailedInvoke(Function<JsonRpcClient, CompletableFuture<?>> invoker,
int errorCode) throws Exception {
var logger = Logger.getLogger(JsonRpcClient.class.getName());
var handler = new RecordingLogHandler();
logger.addHandler(handler);
try (var pair = createSocketPair()) {
CompletableFuture<?> future = invoker.apply(pair.client);

String request = readRpcMessage(pair.serverSide.getInputStream());
long id = MAPPER.readTree(request).get("id").asLong();
writeRpcMessage(pair.serverSide.getOutputStream(), "{\"jsonrpc\":\"2.0\",\"id\":" + id + ",\"error\":{"
+ "\"code\":" + errorCode + ",\"message\":\"Unhandled method connect\"}}");

assertThrows(ExecutionException.class, () -> future.get(5, TimeUnit.SECONDS));
return handler.records;
} finally {
logger.removeHandler(handler);
}
}

/** Matches the JSON-RPC "method not found" rejection from a legacy server. */
private static boolean isMethodNotFound(Throwable cause) {
return cause instanceof JsonRpcException rpcEx && rpcEx.getCode() == -32601;
}

@Test
void testInvokeLogsFailureAtWarningByDefault() throws Exception {
var records = captureLogsForFailedInvoke(client -> client.invoke("connect", Map.of(), JsonNode.class), -32601);

assertTrue(records.stream().anyMatch(r -> r.getLevel() == Level.WARNING),
"Callers that declare no expected failure should still get a WARNING");
}

@Test
void testInvokeDowngradesExpectedFailure() throws Exception {
var records = captureLogsForFailedInvoke(
client -> client.invoke("connect", Map.of(), JsonNode.class, JsonRpcClientTest::isMethodNotFound),
-32601);

assertTrue(records.stream().noneMatch(r -> r.getLevel().intValue() >= Level.WARNING.intValue()),
"A failure the caller expects and recovers from must not be logged at WARNING");
}

@Test
void testInvokeKeepsWarningForUnexpectedFailure() throws Exception {
var records = captureLogsForFailedInvoke(
client -> client.invoke("connect", Map.of(), JsonNode.class, JsonRpcClientTest::isMethodNotFound),
-32603);

assertTrue(records.stream().anyMatch(r -> r.getLevel() == Level.WARNING),
"A failure the predicate rejects must still be logged at WARNING");
}
}