refactor: replace GithubPanicErrorReport with a server-backed ExceptionReporter

.github/workflows/ci.yml

@@ -13,6 +13,9 @@ jobs:
   build:
 
     runs-on: ${{ matrix.os }}
+    # Without an explicit limit a hung test JVM keeps the job alive for the 6 hour default and the
+    # logs are never published, which makes such hangs impossible to diagnose.
+    timeout-minutes: 45
     strategy:
       matrix:
         os: [ubuntu-latest, macos-latest, windows-latest]

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,122 @@
+// 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 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);
+
+    try (ExceptionReporter reporter = new ExceptionReporter()) {
+      reporter.setSink(exception -> {
+        reporterThread.set(Thread.currentThread());
+        reported.countDown();
+      });
+      reporter.report(new IllegalStateException("test"));
+
+      assertTrue(reported.await(5, TimeUnit.SECONDS));
+      assertNotEquals(callerThread, reporterThread.get());
+    }
+  }
+
+  @Test
+  void testReport_withoutSinkIsDiscarded() {
+    AtomicInteger reportCount = new AtomicInteger();
+    try (ExceptionReporter reporter = new ExceptionReporter()) {
+      reporter.report(new IllegalStateException("test"));
+      reporter.setSink(exception -> reportCount.incrementAndGet());
+
+      assertEquals(0, reportCount.get());
+    }
+  }
+
+  @Test
+  void testSetSink_afterCloseIsIgnored() {
+    AtomicInteger reportCount = new AtomicInteger();
+    ExceptionReporter reporter = new ExceptionReporter();
+    reporter.close();
+
+    reporter.setSink(exception -> reportCount.incrementAndGet());
+    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);
+
+    try (ExceptionReporter reporter = new ExceptionReporter(1)) {
+      reporter.setSink(exception -> {
+        int count = reportCount.incrementAndGet();
+        if (count == 1) {
+          firstReportStarted.countDown();
+          try {
+            releaseFirstReport.await();
+          } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+          }
+        } else {
+          secondReportFinished.countDown();
+        }
+      });
+      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 {
+    ExceptionReporter reporter = new ExceptionReporter();
+    reporter.setSink(exception -> {
+    });
+    AtomicReference<Throwable> failure = new AtomicReference<>();
+    CountDownLatch finished = new CountDownLatch(1);
+
+    // report() reads 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/LsStreamConnectionProviderTests.java

@@ -5,8 +5,11 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 import java.io.IOException;
+import java.util.List;
 
 import org.eclipse.core.runtime.preferences.IEclipsePreferences;
 import org.eclipse.core.runtime.preferences.InstanceScope;
@@ -52,11 +55,41 @@ void testInitializationIgnoresLegacyWorkspaceContextPreference() {
 
   @Test
   void testStartLanguageServer() throws IOException {
-    LsStreamConnectionProvider provider = new LsStreamConnectionProvider();
+    TestableLsStreamConnectionProvider provider = new TestableLsStreamConnectionProvider();
+    Process process;
     try {
       provider.start();
+      process = provider.process();
+      assertNotNull(process, "starting the provider must create a language server process");
     } finally {
       provider.stop();
     }
+
+    assertFalse(process.isAlive(), "the language server process must not outlive stop()");
+  }
+
+  /**
+   * A language server that inherits this JVM's stderr keeps that file descriptor open for as long as
+   * it lives. When the server outlives the IDE - which happens because LSP4E tears servers down
+   * asynchronously - whoever reads the other end of that stream waits forever; in a Tycho test fork
+   * that reader is Maven, and the build hangs long after the tests have finished.
+   */
+  @Test
+  void testCreateProcessBuilderDoesNotInheritErrorStream() {
+    LsStreamConnectionProvider provider = new LsStreamConnectionProvider();
+    provider.setCommands(List.of("copilot-language-server", "--stdio"));
+
+    ProcessBuilder builder = provider.createProcessBuilder();
+
+    assertNotEquals(ProcessBuilder.Redirect.INHERIT, builder.redirectError());
+  }
+
+  /**
+   * Exposes the language server process so tests can assert on the real operating system process.
+   */
+  private static final class TestableLsStreamConnectionProvider extends LsStreamConnectionProvider {
+    Process process() {
+      return getProcess();
+    }
   }
 }

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"