Don't block IceAdapter.start() on telemetry websocket connect (fixes #42)

TelemetryDebugger.startupComplete() called websocketClient.connectBlocking()
synchronously on the IceAdapter startup thread, with no timeout. Telemetry
is a debug-only observability channel - its data flow is strictly outgoing
(see ice-adapter/src/main/java/com/faforever/iceadapter/telemetry/, all
messages implement OutgoingMessageV1) and no game logic depends on it being
connected - so it should never gate peer connectivity.

When the telemetry server is silently unreachable (TCP layer up but app
layer hung - for example an alive load balancer fronting a dead backend),
the WebSocket Upgrade handshake never receives an HTTP 101 response.
connectBlocking() then waits until TCP keepalive eventually kills the
socket, about two hours later. During that whole window IceAdapter.start()
is blocked, the FAF client's adapter-ready orchestration trips its own
timeouts, and the user-visible symptom is "the ice adapter doesn't connect
to other players".

Fix: run the connect on a virtual thread so startup returns immediately.
The pre-existing reconnect loop in sendingLoop() already handles transient
drops, and queued messages catch up once the socket opens because the
messageQueue is unbounded.

Also adds the missing return after the InterruptedException catch
(previously fell through to sendMessage on an interrupted thread) and
re-asserts the interrupt flag, per Java best practice.

Fixes #42

Author: juerghegglin

ice-adapter/src/main/java/com/faforever/iceadapter/debug/TelemetryDebugger.java

@@ -28,6 +28,20 @@
 import org.java_websocket.client.WebSocketClient;
 import org.java_websocket.handshake.ServerHandshake;
 
+/**
+ * {@link Debugger} implementation that streams ICE adapter lifecycle, GPGNet, and per-peer events
+ * to the FAF telemetry server over a WebSocket.
+ *
+ * <p>The data flow is strictly outgoing: every payload type in
+ * {@code com.faforever.iceadapter.telemetry} implements {@link OutgoingMessageV1}, and the only
+ * inbound handler ({@link WebSocketClient#onMessage(String)}) just logs what arrives. As a result
+ * telemetry is a debug-only observability channel and must never gate core ICE adapter behaviour.
+ *
+ * <p>Outgoing messages are decoupled from the calling thread by a
+ * {@link LinkedBlockingQueue} drained by {@link #sendingLoop()} on a dedicated virtual thread.
+ * The queue is unbounded, so messages produced before the socket opens (or during a transient
+ * drop) are buffered and flushed on (re)connect.
+ */
 @Slf4j
 public class TelemetryDebugger implements Debugger, AutoCloseable {
     private final WebSocketClient websocketClient;
@@ -38,6 +52,16 @@ public class TelemetryDebugger implements Debugger, AutoCloseable {
 
     private final Thread sendingLoopThread;
 
+    /**
+     * Registers this debugger with {@link Debug}, builds the telemetry WebSocket client targeting
+     * {@code <telemetryServer>/adapter/v1/game/<gameId>/player/<playerId>}, and starts the virtual
+     * thread that drains the outgoing message queue. The connection itself is not opened here; see
+     * {@link #startupComplete()}.
+     *
+     * @param telemetryServer base URI of the telemetry server (e.g. {@code wss://...}).
+     * @param gameId          FAF game id this adapter instance belongs to.
+     * @param playerId        FAF player id this adapter instance belongs to.
+     */
     public TelemetryDebugger(String telemetryServer, int gameId, int playerId) {
         Debug.register(this);
 
@@ -49,21 +73,38 @@ public TelemetryDebugger(String telemetryServer, int gameId, int playerId) {
                 playerId);
 
         websocketClient = new WebSocketClient(uri) {
+            /** Logs that the WebSocket handshake completed. The server handshake metadata is unused. */
             @Override
             public void onOpen(ServerHandshake handshakedata) {
                 log.info("Telemetry websocket opened");
             }
 
+            /**
+             * Logs an inbound telemetry message.
+             *
+             * <p>Telemetry is one-way (adapter -&gt; server) by design, so receiving a message is
+             * unusual and worth surfacing in logs but never acted upon by the adapter.
+             */
             @Override
             public void onMessage(String message) {
                 log.info("Telemetry websocket message: {}", message);
             }
 
+            /** Logs that the WebSocket has been closed, including the reason reported by the peer. */
             @Override
             public void onClose(int code, String reason, boolean remote) {
                 log.info("Telemetry websocket closed (reason: {})", reason);
             }
 
+            /**
+             * Handles errors raised by the WebSocket client.
+             *
+             * <p>A {@link ConnectException} indicates the telemetry server is unreachable; in that
+             * case the surrounding {@link TelemetryDebugger} is removed so subsequent
+             * {@code debug().*} calls iterate an empty debugger list and become no-ops. All other
+             * exceptions are logged but the debugger is kept registered so transient errors don't
+             * disable telemetry permanently.
+             */
             @Override
             public void onError(Exception ex) {
                 if (ex instanceof ConnectException) {
@@ -81,6 +122,16 @@ public void onError(Exception ex) {
         sendingLoopThread = Thread.ofVirtual().name("sendingLoop").start(this::sendingLoop);
     }
 
+    /**
+     * Enqueues an outgoing telemetry message for transmission by {@link #sendingLoop()}.
+     *
+     * <p>The queue is unbounded and the put is interruptible; if the calling thread is interrupted
+     * while waiting (which the {@link LinkedBlockingQueue} does not in practice for an unbounded
+     * queue) the {@link InterruptedException} is rethrown wrapped as a {@link RuntimeException} so
+     * callers do not need to declare it.
+     *
+     * @param message message to enqueue.
+     */
     private void sendMessage(OutgoingMessageV1 message) {
         try {
             messageQueue.put(message);
@@ -89,6 +140,16 @@ private void sendMessage(OutgoingMessageV1 message) {
         }
     }
 
+    /**
+     * Loop body of the dedicated {@code sendingLoop} virtual thread.
+     *
+     * <p>Blocks on {@link BlockingQueue#take()}, serializes each message to JSON via Jackson, and
+     * sends it over the WebSocket. If the socket has dropped, {@link WebSocketClient#reconnectBlocking()}
+     * is called before the message is sent so transient drops are transparent to producers.
+     *
+     * <p>Exits cleanly on interrupt; other exceptions are logged and the loop continues so a single
+     * malformed message cannot stall the entire telemetry channel.
+     */
     @SneakyThrows
     private void sendingLoop() {
         while (!Thread.currentThread().isInterrupted()) {
@@ -113,39 +174,97 @@ private void sendingLoop() {
         }
     }
 
+    /**
+     * Connects to the telemetry websocket server asynchronously and registers this adapter as a peer
+     * once the connection is established.
+     *
+     * <p>Telemetry is a debug-only, strictly outgoing observability channel; no game logic depends
+     * on it being connected. The connect is therefore performed on a virtual thread so that
+     * {@code IceAdapter.start()} returns immediately and peer connectivity is never gated on the
+     * telemetry server being reachable.
+     *
+     * <p>Previously this method called {@code websocketClient.connectBlocking()} synchronously on
+     * the startup thread with no timeout. When the telemetry server's TCP layer was reachable but
+     * its application layer was hung (for example an alive load balancer fronting a dead backend),
+     * the WebSocket Upgrade response never arrived and startup blocked until TCP keepalive killed
+     * the socket roughly two hours later. The user-visible symptom was "the ice adapter doesn't
+     * connect to other players" because the FAF client's adapter-ready orchestration tripped its
+     * own timeouts long before {@code start()} returned.
+     *
+     * <p>Failure handling:
+     * <ul>
+     *   <li>If {@code connectBlocking()} returns {@code false}, this debugger is removed via
+     *       {@link Debug#remove(Debugger)} and no further telemetry is queued.</li>
+     *   <li>If the connect thread is interrupted, the debugger is removed, the failure is logged,
+     *       and the interrupt flag is re-asserted on the current thread per Java best practice.</li>
+     *   <li>On a successful connect, a {@link RegisterAsPeer} message is enqueued. The pre-existing
+     *       reconnect logic in {@link #sendingLoop()} handles transient drops; queued messages catch
+     *       up once the socket re-opens because {@code messageQueue} is unbounded.</li>
+     * </ul>
+     *
+     * @see <a href="https://github.com/FAForever/java-ice-adapter/issues/42">Issue #42</a>
+     */
     @Override
     public void startupComplete() {
-        try {
-            if (!websocketClient.connectBlocking()) {
+        Thread.ofVirtual().name("telemetry-connect").start(() -> {
+            try {
+                if (!websocketClient.connectBlocking()) {
+                    Debug.remove(this);
+                    return;
+                }
+            } catch (InterruptedException e) {
                 Debug.remove(this);
+                log.error("Failed to connect to telemetry websocket", e);
+                Thread.currentThread().interrupt();
                 return;
             }
-        } catch (InterruptedException e) {
-            Debug.remove(this);
-            log.error("Failed to connect to telemetry websocket", e);
-        }
 
-        sendMessage(new RegisterAsPeer(
-                UUID.randomUUID(), "java-ice-adapter/" + IceAdapter.getVersion(), IceAdapter.getLogin()));
+            sendMessage(new RegisterAsPeer(
+                    UUID.randomUUID(), "java-ice-adapter/" + IceAdapter.getVersion(), IceAdapter.getLogin()));
+        });
     }
 
+    /**
+     * Logs that the JSON-RPC server has started and arranges to log again once a peer connects.
+     *
+     * <p>No telemetry message is sent here because RPC peer state is not part of the v1 telemetry
+     * protocol; this hook exists purely for local diagnostics.
+     *
+     * @param peerFuture future that completes with the connected RPC peer.
+     */
     @Override
     public void rpcStarted(CompletableFuture<JJsonPeer> peerFuture) {
         log.info("RPC started");
         peerFuture.thenAccept(peer -> log.info("RPC connected"));
     }
 
+    /**
+     * Sends an {@link UpdateGpgnetState} indicating the GPGNet server is up and waiting for the game
+     * client to connect.
+     */
     @Override
     public void gpgnetStarted() {
         sendMessage(new UpdateGpgnetState(UUID.randomUUID(), "WAITING_FOR_GAME"));
     }
 
+    /**
+     * Sends an {@link UpdateGpgnetState} reflecting the current GPGNet connection state -
+     * {@code GAME_CONNECTED} when {@link GPGNetServer#isConnected()} is {@code true}, otherwise
+     * {@code WAITING_FOR_GAME}.
+     */
     @Override
     public void gpgnetConnectedDisconnected() {
         sendMessage(new UpdateGpgnetState(
                 UUID.randomUUID(), GPGNetServer.isConnected() ? "GAME_CONNECTED" : "WAITING_FOR_GAME"));
     }
 
+    /**
+     * Sends an {@link UpdateGameState} carrying the latest GPGNet game state.
+     *
+     * @throws IllegalStateException if {@link GPGNetServer#getGameState()} is empty - the contract
+     *                               of this hook is that it is invoked on a state transition, so
+     *                               an absent state would indicate a programming error upstream.
+     */
     @Override
     public void gameStateChanged() {
         sendMessage(new UpdateGameState(
@@ -154,17 +273,40 @@ public void gameStateChanged() {
                         .orElseThrow(() -> new IllegalStateException("gameState must not change to null"))));
     }
 
+    /**
+     * Sends a {@link ConnectToPeer} event when the adapter starts attempting to reach a peer.
+     *
+     * @param id          remote player id.
+     * @param login       remote player login.
+     * @param localOffer  {@code true} if this adapter generated the SDP offer, {@code false} if it
+     *                    is the answering side.
+     */
     @Override
     public void connectToPeer(int id, String login, boolean localOffer) {
         sendMessage(new ConnectToPeer(UUID.randomUUID(), id, login, localOffer));
     }
 
+    /**
+     * Sends a {@link DisconnectFromPeer} event and drops any rate-limiter state held for the peer
+     * so a future reconnect starts from a clean slate.
+     *
+     * @param id remote player id.
+     */
     @Override
     public void disconnectFromPeer(int id) {
         peerRateLimiter.remove(id);
         sendMessage(new DisconnectFromPeer(UUID.randomUUID(), id));
     }
 
+    /**
+     * Sends an {@link UpdatePeerState} carrying the current ICE state and the candidate types of
+     * the currently selected pair (local and remote), if any.
+     *
+     * <p>Unlike {@link #peerConnectivityUpdate(Peer)} this method is not rate-limited: ICE state
+     * transitions are infrequent and each one is independently meaningful for diagnostics.
+     *
+     * @param peer peer whose ICE state changed.
+     */
     @Override
     public void peerStateChanged(Peer peer) {
         sendMessage(new UpdatePeerState(
@@ -183,6 +325,16 @@ public void peerStateChanged(Peer peer) {
                         .orElse(null)));
     }
 
+    /**
+     * Sends an {@link UpdatePeerConnectivity} with the latest RTT and last-packet-received timestamp
+     * for the given peer.
+     *
+     * <p>Rate-limited to one update per second per peer via a per-peer Guava {@link RateLimiter},
+     * because connectivity samples can fire at the underlying STUN keepalive cadence and would
+     * otherwise dominate the telemetry stream.
+     *
+     * @param peer peer whose connectivity sample is being reported.
+     */
     @Override
     public void peerConnectivityUpdate(Peer peer) {
         if (!peerRateLimiter
@@ -210,6 +362,13 @@ public void peerConnectivityUpdate(Peer peer) {
                         .orElse(null)));
     }
 
+    /**
+     * Sends an {@link UpdateCoturnList} reporting the COTURN servers currently known to the adapter.
+     * The first server's host is included as a convenience so the telemetry UI can display the
+     * "primary" relay without pulling the whole list apart.
+     *
+     * @param servers current COTURN server list (may be empty).
+     */
     @Override
     public void updateCoturnList(Collection<CoturnServer> servers) {
         sendMessage(new UpdateCoturnList(
@@ -218,6 +377,13 @@ public void updateCoturnList(Collection<CoturnServer> servers) {
                 servers));
     }
 
+    /**
+     * Interrupts the sending loop so the dedicated virtual thread can exit cleanly.
+     *
+     * <p>Implements {@link AutoCloseable} so callers can manage telemetry lifetime with
+     * try-with-resources. The WebSocket itself is not closed here - the underlying
+     * {@link WebSocketClient} owns its own close lifecycle and the JVM will tear it down on exit.
+     */
     @Override
     public void close() {
         sendingLoopThread.interrupt();