fix: avoid exception reporting deadlocks

CONTEXT.md

@@ -36,3 +36,8 @@ project's files are semantically indexed.
 ### Workspace instructions
 Custom instructions associated with workspace folders and loaded into chat.
 They are distinct from semantic search over project files.
+
+### Exception report
+A best-effort diagnostic record of an unexpected failure. Exception reporting
+must not start or delay the language server and may be dropped when no running
+server connection is available.

com.microsoft.copilot.eclipse.core.test/META-INF/MANIFEST.MF

@@ -20,7 +20,6 @@ Require-Bundle: com.microsoft.copilot.eclipse.core;bundle-version="0.20.0",
  org.eclipse.lsp4j,
  org.eclipse.core.jobs,
  org.eclipse.equinox.common,
- org.eclipse.core.net,
  org.eclipse.core.resources,
  org.eclipse.text,
  org.eclipse.core.runtime

com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/logger/ExceptionReporterTests.java

@@ -0,0 +1,135 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+package com.microsoft.copilot.eclipse.core.logger;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Consumer;
+
+import org.junit.jupiter.api.Test;
+
+class ExceptionReporterTests {
+
+  @Test
+  void testReport_dispatchesOnReporterThread() throws InterruptedException {
+    Thread callerThread = Thread.currentThread();
+    AtomicReference<Thread> reporterThread = new AtomicReference<>();
+    CountDownLatch reported = new CountDownLatch(1);
+    Consumer<Throwable> sink = exception -> {
+      reporterThread.set(Thread.currentThread());
+      reported.countDown();
+    };
+
+    try (ExceptionReporter reporter = new ExceptionReporter(() -> sink)) {
+      reporter.report(new IllegalStateException("test"));
+
+      assertTrue(reported.await(5, TimeUnit.SECONDS));
+      assertNotEquals(callerThread, reporterThread.get());
+    }
+  }
+
+  @Test
+  void testReport_resolvesTheSinkOnEveryReport() throws InterruptedException {
+    AtomicInteger reportCount = new AtomicInteger();
+    CountDownLatch reported = new CountDownLatch(1);
+    AtomicReference<Consumer<Throwable>> sink = new AtomicReference<>();
+
+    try (ExceptionReporter reporter = new ExceptionReporter(sink::get)) {
+      // Nothing consumes reports yet, so this one is dropped rather than queued.
+      reporter.report(new IllegalStateException("no sink yet"));
+
+      sink.set(exception -> {
+        reportCount.incrementAndGet();
+        reported.countDown();
+      });
+      reporter.report(new IllegalStateException("test"));
+
+      assertTrue(reported.await(5, TimeUnit.SECONDS));
+      assertEquals(1, reportCount.get());
+    }
+  }
+
+  @Test
+  void testReport_afterCloseIsDiscarded() {
+    AtomicInteger reportCount = new AtomicInteger();
+    // The supplier keeps handing out a live sink after close(), so the shut down executor is what
+    // has to drop the work. That is why the reporter no longer tracks a sink of its own.
+    Consumer<Throwable> sink = exception -> reportCount.incrementAndGet();
+    ExceptionReporter reporter = new ExceptionReporter(() -> sink);
+    reporter.close();
+
+    reporter.report(new IllegalStateException("test"));
+
+    assertEquals(0, reportCount.get());
+  }
+
+  @Test
+  void testReport_discardsWhenQueueIsFull() throws InterruptedException {
+    AtomicInteger reportCount = new AtomicInteger();
+    CountDownLatch firstReportStarted = new CountDownLatch(1);
+    CountDownLatch releaseFirstReport = new CountDownLatch(1);
+    CountDownLatch secondReportFinished = new CountDownLatch(1);
+    Consumer<Throwable> sink = exception -> {
+      int count = reportCount.incrementAndGet();
+      if (count == 1) {
+        firstReportStarted.countDown();
+        try {
+          releaseFirstReport.await();
+        } catch (InterruptedException e) {
+          Thread.currentThread().interrupt();
+        }
+      } else {
+        secondReportFinished.countDown();
+      }
+    };
+
+    try (ExceptionReporter reporter = new ExceptionReporter(() -> sink, 1)) {
+      reporter.report(new IllegalStateException("first"));
+      assertTrue(firstReportStarted.await(5, TimeUnit.SECONDS));
+
+      reporter.report(new IllegalStateException("second"));
+      reporter.report(new IllegalStateException("discarded"));
+      releaseFirstReport.countDown();
+
+      assertTrue(secondReportFinished.await(5, TimeUnit.SECONDS));
+      assertEquals(2, reportCount.get());
+    }
+  }
+
+  @Test
+  void testReport_concurrentWithCloseDoesNotThrow() throws InterruptedException {
+    Consumer<Throwable> sink = exception -> {
+    };
+    ExceptionReporter reporter = new ExceptionReporter(() -> sink);
+    AtomicReference<Throwable> failure = new AtomicReference<>();
+    CountDownLatch finished = new CountDownLatch(1);
+
+    // report() resolves the sink and submits to the executor in two steps, so close() can land in
+    // between. Reporting runs on the platform logging thread and must never propagate a failure
+    // there, so the rejected submission has to be discarded instead.
+    Thread reporting = new Thread(() -> {
+      try {
+        for (int i = 0; i < 2000; i++) {
+          reporter.report(new IllegalStateException("test"));
+        }
+      } catch (Throwable e) {
+        failure.set(e);
+      } finally {
+        finished.countDown();
+      }
+    });
+    reporting.start();
+    reporter.close();
+
+    assertTrue(finished.await(10, TimeUnit.SECONDS));
+    assertNull(failure.get());
+  }
+}

com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageServerConnectionTests.java

@@ -0,0 +1,126 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+package com.microsoft.copilot.eclipse.core.lsp;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
+
+import org.eclipse.lsp4e.LanguageServerWrapper;
+import org.eclipse.lsp4j.services.LanguageServer;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import com.microsoft.copilot.eclipse.core.lsp.protocol.TelemetryExceptionParams;
+
+@ExtendWith(MockitoExtension.class)
+class CopilotLanguageServerConnectionTests {
+
+  @Mock
+  private LanguageServerWrapper languageServerWrapper;
+  @Mock
+  private CopilotLanguageServer languageServer;
+
+  private final AtomicReference<Function<LanguageServer, ? extends CompletableFuture<Void>>> sinkInitializer =
+      new AtomicReference<>();
+  private CopilotLanguageServerConnection connection;
+
+  @BeforeEach
+  void setUp() {
+    when(languageServerWrapper.<Void>execute(any())).thenAnswer(invocation -> {
+      sinkInitializer.set(invocation.getArgument(0));
+      return new CompletableFuture<Void>();
+    });
+    connection = new CopilotLanguageServerConnection(languageServerWrapper);
+  }
+
+  @Test
+  void testSendExceptionTelemetry_beforeInitialization_doesNotReenterWrapper() {
+    assertNull(connection.sendExceptionTelemetry(new IllegalStateException("test")).join());
+
+    verify(languageServerWrapper, times(1)).execute(any());
+    verify(languageServer, never()).sendExceptionTelemetry(any());
+  }
+
+  @Test
+  void testSendExceptionTelemetry_afterInitialization_usesCachedSink() {
+    sinkInitializer.get().apply(languageServer).join();
+    when(languageServer.sendExceptionTelemetry(any())).thenReturn(CompletableFuture.completedFuture(null));
+
+    assertNull(connection.sendExceptionTelemetry(new IllegalStateException("test")).join());
+
+    ArgumentCaptor<TelemetryExceptionParams> paramsCaptor = ArgumentCaptor.forClass(TelemetryExceptionParams.class);
+    verify(languageServer).sendExceptionTelemetry(paramsCaptor.capture());
+    verify(languageServerWrapper, times(1)).execute(any());
+    assertEquals(1, paramsCaptor.getValue().getExceptionDetail().size());
+  }
+
+  @Test
+  void testSendExceptionTelemetry_afterFailureOnActiveServerKeepsSink() {
+    sinkInitializer.get().apply(languageServer).join();
+    when(languageServerWrapper.isActive()).thenReturn(true);
+    when(languageServer.sendExceptionTelemetry(any()))
+        .thenReturn(CompletableFuture.failedFuture(new IllegalStateException("transient")));
+
+    assertNull(connection.sendExceptionTelemetry(new IllegalStateException("first")).join());
+    assertNull(connection.sendExceptionTelemetry(new IllegalStateException("second")).join());
+
+    verify(languageServer, times(2)).sendExceptionTelemetry(any());
+  }
+
+  @Test
+  void testSendExceptionTelemetry_afterFailureOnStoppedServerClearsSink() {
+    sinkInitializer.get().apply(languageServer).join();
+    when(languageServerWrapper.isActive()).thenReturn(false);
+    when(languageServer.sendExceptionTelemetry(any()))
+        .thenReturn(CompletableFuture.failedFuture(new IllegalStateException("closed")));
+
+    assertNull(connection.sendExceptionTelemetry(new IllegalStateException("first")).join());
+    assertNull(connection.sendExceptionTelemetry(new IllegalStateException("second")).join());
+
+    verify(languageServer, times(1)).sendExceptionTelemetry(any());
+  }
+
+  @Test
+  void testStop_clearsExceptionSinkBeforeStoppingWrapper() {
+    sinkInitializer.get().apply(languageServer).join();
+
+    connection.stop();
+    assertNull(connection.sendExceptionTelemetry(new IllegalStateException("test")).join());
+
+    verify(languageServerWrapper).stop();
+    verify(languageServer, never()).sendExceptionTelemetry(any());
+  }
+
+  @Test
+  void testStop_beforeInitializationPreventsLateSinkRegistration() {
+    connection.stop();
+    sinkInitializer.get().apply(languageServer).join();
+
+    assertNull(connection.sendExceptionTelemetry(new IllegalStateException("test")).join());
+
+    verify(languageServerWrapper).stop();
+    verify(languageServer, never()).sendExceptionTelemetry(any());
+  }
+
+  @Test
+  void testStop_isIdempotent() {
+    connection.stop();
+    connection.stop();
+
+    verify(languageServerWrapper, times(1)).stop();
+  }
+}

com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/protocol/TelemetryExceptionParamsTests.java

@@ -0,0 +1,36 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+package com.microsoft.copilot.eclipse.core.lsp.protocol;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import org.junit.jupiter.api.Test;
+
+class TelemetryExceptionParamsTests {
+
+  @Test
+  void testConstructor_withSourceFileBuildsPath() {
+    var params = new TelemetryExceptionParams(exceptionWithFrame("SampleClass.java"));
+
+    assertEquals("com/microsoft/copilot/SampleClass.java", firstFrameFilename(params));
+  }
+
+  @Test
+  void testConstructor_withoutSourceFileKeepsClassName() {
+    var params = new TelemetryExceptionParams(exceptionWithFrame(null));
+
+    assertEquals("com/microsoft/copilot/SampleClass", firstFrameFilename(params));
+  }
+
+  private static Throwable exceptionWithFrame(String fileName) {
+    var exception = new IllegalStateException("boom");
+    exception.setStackTrace(new StackTraceElement[] {
+        new StackTraceElement("com.microsoft.copilot.SampleClass", "run", fileName, 42) });
+    return exception;
+  }
+
+  private static String firstFrameFilename(TelemetryExceptionParams params) {
+    return params.getExceptionDetail().get(0).getStacktrace()[0].getFilename();
+  }
+}

com.microsoft.copilot.eclipse.core/META-INF/MANIFEST.MF

@@ -41,12 +41,9 @@ Require-Bundle: org.eclipse.lsp4e;bundle-version="0.18.1",
  org.eclipse.jface.text;bundle-version="3.24.200",
  com.google.gson;bundle-version="2.10.1",
  org.eclipse.wildwebdeveloper.embedder.node;bundle-version="1.0.3";resolution:=optional,
- org.eclipse.core.net;bundle-version="1.5.200",
  org.eclipse.core.resources;bundle-version="3.20.0",
  org.eclipse.core.filesystem;bundle-version="1.10.200",
  org.eclipse.core.runtime;bundle-version="[3.30.0,4.0.0)",
- org.apache.httpcomponents.client5.httpclient5;bundle-version="5.2.1",
- org.apache.httpcomponents.core5.httpcore5;bundle-version="5.2.3",
  org.osgi.service.event;bundle-version="1.4.1",
  org.eclipse.e4.core.services;bundle-version="2.4.200",
  org.eclipse.e4.core.contexts;bundle-version="1.12.400"