From 8e8242babb9b0c08a1ed63fd36e864e3db94f3de Mon Sep 17 00:00:00 2001 From: Sean Champ Date: Sun, 16 Oct 2022 02:17:30 -0700 Subject: [PATCH] Additional logging for launcher (#25) * Adding logging for exceptions in config access and commands via launcher ConfigHelper - new class - defining two static delegate methods for accessing a string config property, with logging for exceptions during config access LaunchHelper - new class - defining a createJob method. This method provides a central implementation for calls that were implemented in both BundleGemRunShortcut and LaunchConfigurationDelegate, both creating a runnable Job spec for the DebugPlugin. - the createJob method will log instead of throwing CoreException, if the process launcher is cancelled during createJob or if the external process exits with a non-zero exit status. ReadaptDebugDelegate - moving two constants to the class scope - using ConfigHelper - using LogHelper to log any exception when launching readapt - updating the launch method signature, logging instead of throwing CoreException BundleGemRunShortcut - using LaunchHelper RubyRunDelegate - using ConfigHelper, LaunchHelper LogHelper - creating a constant for the bundle symbolic name - creating two generic 'log' methods, here used within the 'info', 'error', and additional 'cancelled' methods - adding an 'error' method accepting only a string arg - adding a 'cancelled' method accepting only a string arg This changeset updates previous plugin behaviors. For processes initialized via BundleGemRunShortcut or LaunchConfigurationDelegate, a logging message will be created only on the following events: - exception during process launch - cancellation during process launch - non-zero process exit * Updating patch branch - declaring LOGNAME as a final string * Updating config calls, logging in ReadaptDebugDelegate ReadaptDebugDelegate - Using the getConfigString default value in all calls to getConfigString - joining elements of READAPT_STDIO for log string --- .../launch/debug/ReadaptDebugDelegate.java | 26 ++++++--- .../launch/run/BundleGemRunShortcut.java | 9 +-- .../launch/run/RubyRunDelegate.java | 26 ++++----- .../utils/ConfigHelper.java | 21 +++++++ .../utils/LaunchHelper.java | 58 +++++++++++++++++++ .../eclipse_solargraph/utils/LogHelper.java | 21 ++++++- 6 files changed, 129 insertions(+), 32 deletions(-) create mode 100644 eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/utils/ConfigHelper.java create mode 100644 eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/utils/LaunchHelper.java diff --git a/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/launch/debug/ReadaptDebugDelegate.java b/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/launch/debug/ReadaptDebugDelegate.java index b2cc70f..6a4eb21 100644 --- a/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/launch/debug/ReadaptDebugDelegate.java +++ b/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/launch/debug/ReadaptDebugDelegate.java @@ -34,15 +34,19 @@ import io.github.pyvesb.eclipse_solargraph.launch.RubyLaunchShortcut; import io.github.pyvesb.eclipse_solargraph.preferences.PreferencePage; +import io.github.pyvesb.eclipse_solargraph.utils.ConfigHelper; import io.github.pyvesb.eclipse_solargraph.utils.GemHelper; +import io.github.pyvesb.eclipse_solargraph.utils.LogHelper; public class ReadaptDebugDelegate extends DSPLaunchDelegate { private static final AtomicBoolean HAS_UPDATED_READAPT = new AtomicBoolean(); + private static final List READAPT_STDIO = List.of("stdio"); + private static final Long READAPT_UPDATE_DELAY = 5000L; + @Override - public void launch(ILaunchConfiguration configuration, String mode, ILaunch launch, IProgressMonitor monitor) - throws CoreException { + public void launch(ILaunchConfiguration configuration, String mode, ILaunch launch, IProgressMonitor monitor) { String readaptPath = READAPT_PATH.getValue(); if (readaptPath == null || !new File(readaptPath).exists()) { displayNotFoundWarning(); @@ -50,17 +54,23 @@ public void launch(ILaunchConfiguration configuration, String mode, ILaunch laun } DSPLaunchDelegateLaunchBuilder builder = new DSPLaunchDelegateLaunchBuilder(configuration, mode, launch, monitor); - builder.setLaunchDebugAdapter("\"" + readaptPath + "\"", List.of("stdio")); + builder.setLaunchDebugAdapter(readaptPath, READAPT_STDIO); builder.setMonitorDebugAdapter(DEBUG_READAPT.getValue()); builder.setDspParameters(Map.of( - "program", configuration.getAttribute(RubyLaunchShortcut.SCRIPT, ""), - "runtimeArgs", configuration.getAttribute(RubyLaunchShortcut.ARGUMENTS, ""), - "cwd", configuration.getAttribute(DebugPlugin.ATTR_WORKING_DIRECTORY, ""), + "program", ConfigHelper.getConfigString(configuration, RubyLaunchShortcut.SCRIPT), + "runtimeArgs", ConfigHelper.getConfigString(configuration, RubyLaunchShortcut.ARGUMENTS), + "cwd", ConfigHelper.getConfigString(configuration, DebugPlugin.ATTR_WORKING_DIRECTORY), "request", "launch")); - super.launch(builder); + try { + super.launch(builder); + } catch (CoreException e) { + String msg = "Exception when launching readapt: " + readaptPath + " " + String.join(" ", READAPT_STDIO); + LogHelper.error(msg); + return; + } if (UPDATE_GEM.getValue() && !HAS_UPDATED_READAPT.getAndSet(true)) { - GemHelper.scheduleUpdate("Readapt", 5000L); + GemHelper.scheduleUpdate("Readapt", READAPT_UPDATE_DELAY); } } diff --git a/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/launch/run/BundleGemRunShortcut.java b/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/launch/run/BundleGemRunShortcut.java index 2b51711..4f691b1 100644 --- a/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/launch/run/BundleGemRunShortcut.java +++ b/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/launch/run/BundleGemRunShortcut.java @@ -15,13 +15,12 @@ import java.io.File; import org.eclipse.core.resources.IResource; -import org.eclipse.core.runtime.jobs.Job; import org.eclipse.debug.core.DebugPlugin; import org.eclipse.debug.core.ILaunchManager; import org.eclipse.debug.core.Launch; import io.github.pyvesb.eclipse_solargraph.launch.IResourceLaunchShortcut; -import io.github.pyvesb.eclipse_solargraph.utils.CommandHelper; +import io.github.pyvesb.eclipse_solargraph.utils.LaunchHelper; public class BundleGemRunShortcut implements IResourceLaunchShortcut { @@ -30,12 +29,8 @@ public void launchResource(IResource resource, String mode) { Launch launch = new Launch(null, ILaunchManager.RUN_MODE, null); DebugPlugin.getDefault().getLaunchManager().addLaunch(launch); String command = getBaseCommand(resource); - String[] absolutePlatformCommand = CommandHelper.getAbsolutePlatformCommand(command); File workingDirectory = resource.getLocation().removeLastSegments(1).toFile(); - Job.create("Running " + command, r -> { - Process process = DebugPlugin.exec(absolutePlatformCommand, workingDirectory); - DebugPlugin.newProcess(launch, process, command); - }).schedule(); + LaunchHelper.createJob(launch, command, workingDirectory).schedule(); } private String getBaseCommand(IResource resource) { diff --git a/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/launch/run/RubyRunDelegate.java b/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/launch/run/RubyRunDelegate.java index df52140..d8fadf3 100644 --- a/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/launch/run/RubyRunDelegate.java +++ b/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/launch/run/RubyRunDelegate.java @@ -14,33 +14,29 @@ *******************************************************************************/ package io.github.pyvesb.eclipse_solargraph.launch.run; -import java.io.File; - -import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IProgressMonitor; -import org.eclipse.core.runtime.jobs.Job; import org.eclipse.debug.core.DebugPlugin; import org.eclipse.debug.core.ILaunch; import org.eclipse.debug.core.ILaunchConfiguration; import org.eclipse.debug.core.model.LaunchConfigurationDelegate; import io.github.pyvesb.eclipse_solargraph.launch.RubyLaunchShortcut; -import io.github.pyvesb.eclipse_solargraph.utils.CommandHelper; +import io.github.pyvesb.eclipse_solargraph.utils.ConfigHelper; +import io.github.pyvesb.eclipse_solargraph.utils.LaunchHelper; public class RubyRunDelegate extends LaunchConfigurationDelegate { @Override - public void launch(ILaunchConfiguration configuration, String mode, ILaunch launch, IProgressMonitor monitor) - throws CoreException { - String script = configuration.getAttribute(RubyLaunchShortcut.SCRIPT, ""); - String arguments = configuration.getAttribute(RubyLaunchShortcut.ARGUMENTS, ""); - String workingDirectory = configuration.getAttribute(DebugPlugin.ATTR_WORKING_DIRECTORY, ""); + public void launch(ILaunchConfiguration configuration, String mode, ILaunch launch, IProgressMonitor monitor) { + String script = ConfigHelper.getConfigString(configuration, RubyLaunchShortcut.SCRIPT); + String arguments = ConfigHelper.getConfigString(configuration, RubyLaunchShortcut.ARGUMENTS); + String workingDirectory = ConfigHelper.getConfigString(configuration, DebugPlugin.ATTR_WORKING_DIRECTORY); + if (script == null || arguments == null || workingDirectory == null) { + // a config attribute reader threw exception & was logged + return; + } String command = "ruby " + script + " " + arguments; - String[] absolutePlatformCommand = CommandHelper.getAbsolutePlatformCommand(command); - Job.create("Running " + command, r -> { - Process process = DebugPlugin.exec(absolutePlatformCommand, new File(workingDirectory)); - DebugPlugin.newProcess(launch, process, command); - }).schedule(); + LaunchHelper.createJob(launch, command, workingDirectory).schedule(); } } diff --git a/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/utils/ConfigHelper.java b/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/utils/ConfigHelper.java new file mode 100644 index 0000000..4406d88 --- /dev/null +++ b/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/utils/ConfigHelper.java @@ -0,0 +1,21 @@ +package io.github.pyvesb.eclipse_solargraph.utils; + +import org.eclipse.core.runtime.CoreException; +import org.eclipse.debug.core.ILaunchConfiguration; + +public class ConfigHelper { + + public static String getConfigString(ILaunchConfiguration configuration, String name) { + return getConfigString(configuration, name, ""); + } + + public static String getConfigString(ILaunchConfiguration configuration, String name, String defaultValue) { + try { + return configuration.getAttribute(name, defaultValue); + } catch (CoreException e) { + LogHelper.error("Unable to access configuration attribute: " + name, e); + return null; + } + } + +} diff --git a/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/utils/LaunchHelper.java b/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/utils/LaunchHelper.java new file mode 100644 index 0000000..0ea52e6 --- /dev/null +++ b/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/utils/LaunchHelper.java @@ -0,0 +1,58 @@ +package io.github.pyvesb.eclipse_solargraph.utils; + +import java.io.File; +import java.util.concurrent.CompletableFuture; + +import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.Status; +import org.eclipse.core.runtime.jobs.Job; +import org.eclipse.debug.core.DebugPlugin; +import org.eclipse.debug.core.ILaunch; + +public class LaunchHelper { + + + public static Job createJob(ILaunch launch, String command, String workingDirectory) { + return createJob(launch, command, new File(workingDirectory)); + } + + public static Job createJob(ILaunch launch, String command, File workingDirectory) { + String[] absolutePlatformCommand = CommandHelper.getAbsolutePlatformCommand(command); + return Job.create("Running " + command, r -> { + try { + Process process = DebugPlugin.exec(absolutePlatformCommand, workingDirectory); + if (process == null) { + LogHelper.cancelled("Command cancelled: " + command); + return; + } + CompletableFuture future = process.onExit(); + // initialize a process completion future to log any non-zero process exit + future.defaultExecutor().execute(new Runnable() { + @Override + public void run() { + try { + if (process.isAlive()) { + process.waitFor(); + } + } catch (InterruptedException e) { + LogHelper.cancelled("Process monitor interrupted: " + command); + return; + } + int exc = process.exitValue(); + if (exc != 0) { + String msg = String.format("Process exited non-zero (%d): %s ", exc, command); + LogHelper.error(msg); + } + } + + }); + DebugPlugin.newProcess(launch, process, command); + } catch (CoreException e) { + // CoreException from exec + LogHelper.log(Status.ERROR, "Exception when launching process: " + command, e); + return; + } + }); + } + +};; diff --git a/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/utils/LogHelper.java b/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/utils/LogHelper.java index e77caba..1835d70 100644 --- a/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/utils/LogHelper.java +++ b/eclipse-solargraph-plugin/src/main/java/io/github/pyvesb/eclipse_solargraph/utils/LogHelper.java @@ -22,14 +22,31 @@ public class LogHelper { private static final Bundle BUNDLE = FrameworkUtil.getBundle(LogHelper.class); + private static final String LOGNAME = BUNDLE.getSymbolicName(); private static final ILog LOGGER = Platform.getLog(BUNDLE); + public static void log(int severity, String message) { + LOGGER.log(new Status(severity, LOGNAME, message)); + } + + public static void log(int severity, String message, Throwable throwable) { + LOGGER.log(new Status(severity, LOGNAME, message, throwable)); + } + public static void info(String message) { - LOGGER.log(new Status(IStatus.INFO, BUNDLE.getSymbolicName(), message)); + log(IStatus.INFO, message); } public static void error(String message, Throwable exception) { - LOGGER.log(new Status(IStatus.ERROR, BUNDLE.getSymbolicName(), message, exception)); + log(IStatus.ERROR, message, exception); + } + + public static void error(String message) { + log(IStatus.ERROR, message); + } + + public static void cancelled(String message) { + log(IStatus.CANCEL, message); } private LogHelper() {