Improve the HCR workflow (#335)

* Improve the HCR workflow

Signed-off-by: Jinbo Wang <[email protected]>

* refine the error message

Signed-off-by: Jinbo Wang <[email protected]>

Co-authored-by: Jinbo Wang <[email protected]>

Author: testforstephen

com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/ErrorCode.java

@@ -34,7 +34,8 @@ public enum ErrorCode {
     COMPLETIONS_FAILURE(1017),
     EXCEPTION_INFO_FAILURE(1018),
     EVALUATION_COMPILE_ERROR(2001),
-    EVALUATE_NOT_SUSPENDED_THREAD(2002);
+    EVALUATE_NOT_SUSPENDED_THREAD(2002),
+    HCR_FAILURE(3001);
 
     private int id;
 

com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/handler/HotCodeReplaceHandler.java

@@ -59,8 +59,12 @@ public CompletableFuture<Response> handle(Command command, Arguments arguments,
 
         IHotCodeReplaceProvider provider = context.getProvider(IHotCodeReplaceProvider.class);
 
-        return provider.redefineClasses().thenApply(classNames -> {
+        return provider.redefineClasses().thenCompose(classNames -> {
             response.body = new Responses.RedefineClassesResponse(classNames.toArray(new String[0]));
+            return CompletableFuture.completedFuture(response);
+        }).exceptionally(ex -> {
+            String errorMessage = ex.getCause() != null ? ex.getCause().getMessage() : ex.getMessage();
+            response.body = new Responses.RedefineClassesResponse(new String[0], errorMessage);
             return response;
         });
     }

com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/protocol/Responses.java

@@ -299,12 +299,21 @@ public ExceptionInfoResponse(String exceptionId, String description, ExceptionBr
 
     public static class RedefineClassesResponse extends ResponseBody {
         public String[] changedClasses = new String[0];
+        public String errorMessage = null;
 
         /**
          * Constructor.
          */
         public RedefineClassesResponse(String[] changedClasses) {
+            this(changedClasses, null);
+        }
+
+        /**
+         * Constructor.
+         */
+        public RedefineClassesResponse(String[] changedClasses, String errorMessage) {
             this.changedClasses = changedClasses;
+            this.errorMessage = errorMessage;
         }
     }
 }

com.microsoft.java.debug.plugin/src/main/java/com/microsoft/java/debug/plugin/internal/JavaHotCodeReplaceProvider.java

@@ -26,8 +26,10 @@
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.CompletableFuture;
 import java.util.function.Consumer;
 import java.util.logging.Level;
@@ -53,13 +55,16 @@
 import org.eclipse.jdt.core.util.IClassFileReader;
 import org.eclipse.jdt.core.util.ISourceAttribute;
 import org.eclipse.jdt.internal.core.util.Util;
+import org.eclipse.jdt.ls.core.internal.JobHelpers;
 
 import com.microsoft.java.debug.core.Configuration;
 import com.microsoft.java.debug.core.DebugException;
 import com.microsoft.java.debug.core.DebugSettings;
 import com.microsoft.java.debug.core.DebugUtility;
 import com.microsoft.java.debug.core.IDebugSession;
 import com.microsoft.java.debug.core.StackFrameUtility;
+import com.microsoft.java.debug.core.adapter.AdapterUtils;
+import com.microsoft.java.debug.core.adapter.ErrorCode;
 import com.microsoft.java.debug.core.adapter.HotCodeReplaceEvent;
 import com.microsoft.java.debug.core.adapter.IDebugAdapterContext;
 import com.microsoft.java.debug.core.adapter.IHotCodeReplaceProvider;
@@ -96,9 +101,9 @@ public class JavaHotCodeReplaceProvider implements IHotCodeReplaceProvider, IRes
 
     private PublishSubject<HotCodeReplaceEvent> eventSubject = PublishSubject.<HotCodeReplaceEvent>create();
 
-    private List<IResource> deltaResources = new ArrayList<>();
+    private Set<IResource> deltaResources = new LinkedHashSet<>();
 
-    private List<String> deltaClassNames = new ArrayList<>();
+    private Set<String> deltaClassNames = new LinkedHashSet<>();
 
     /**
      * Visitor for resource deltas.
@@ -300,14 +305,23 @@ public void onClassRedefined(Consumer<List<String>> consumer) {
 
     @Override
     public CompletableFuture<List<String>> redefineClasses() {
+        JobHelpers.waitForBuildJobs(10 * 1000);
         return CompletableFuture.supplyAsync(() -> {
             List<String> classNames = new ArrayList<>();
+            List<IResource> resources = new ArrayList<>();
+            String errorMessage = null;
             synchronized (this) {
                 classNames.addAll(deltaClassNames);
-                doHotCodeReplace(deltaResources, deltaClassNames);
+                resources.addAll(deltaResources);
                 deltaResources.clear();
                 deltaClassNames.clear();
+                errorMessage = doHotCodeReplace(resources, classNames);
             }
+
+            if (!classNames.isEmpty() && errorMessage != null) {
+                throw AdapterUtils.createCompletionException(errorMessage, ErrorCode.HCR_FAILURE);
+            }
+
             return classNames;
         });
     }
@@ -325,27 +339,29 @@ private void publishEvent(HotCodeReplaceEvent.EventType type, String message, Ob
         eventSubject.onNext(new HotCodeReplaceEvent(type, message, data));
     }
 
-    private void doHotCodeReplace(List<IResource> resourcesToReplace, List<String> qualifiedNamesToReplace) {
+    private String doHotCodeReplace(List<IResource> resourcesToReplace, List<String> qualifiedNamesToReplace) {
         if (context == null || currentDebugSession == null) {
-            return;
+            return null;
         }
 
         if (resourcesToReplace == null || qualifiedNamesToReplace == null || qualifiedNamesToReplace.isEmpty()
                 || resourcesToReplace.isEmpty()) {
-            return;
+            return null;
         }
 
         filterNotLoadedTypes(resourcesToReplace, qualifiedNamesToReplace);
         if (qualifiedNamesToReplace.isEmpty()) {
-            return;
+            return null;
             // If none of the changed types are loaded, do nothing.
         }
 
         // Not supported scenario:
         if (!currentDebugSession.getVM().canRedefineClasses()) {
-            return;
+            publishEvent(HotCodeReplaceEvent.EventType.ERROR, "JVM doesn't support hot reload classes");
+            return "JVM doesn't support hot reload classes";
         }
 
+        String errorMessage = null;
         publishEvent(HotCodeReplaceEvent.EventType.STARTING, "Start hot code replacement procedure...");
 
         try {
@@ -367,6 +383,7 @@ private void doHotCodeReplace(List<IResource> resourcesToReplace, List<String> q
 
             if (containsObsoleteMethods()) {
                 publishEvent(HotCodeReplaceEvent.EventType.ERROR, "JVM contains obsolete methods");
+                errorMessage = "JVM contains obsolete methods";
             }
 
             if (currentDebugSession.getVM().canPopFrames() && framesPopped) {
@@ -376,11 +393,13 @@ private void doHotCodeReplace(List<IResource> resourcesToReplace, List<String> q
             }
         } catch (DebugException e) {
             logger.log(Level.SEVERE, "Failed to complete hot code replace: " + e.getMessage(), e);
+            errorMessage = e.getMessage();
         } finally {
             publishEvent(HotCodeReplaceEvent.EventType.END, "Completed hot code replace", qualifiedNamesToReplace);
+            threadFrameMap.clear();
         }
 
-        threadFrameMap.clear();
+        return errorMessage;
     }
 
     private void filterNotLoadedTypes(List<IResource> resources, List<String> qualifiedNames) {