improve error handling (#126)

newrelic · Aug 1, 2022 · d842b96 · d842b96
1 parent d82187e
commit d842b96
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 27 deletions.
diff --git a/src/main/java/org/newrelic/nrjmx/v2/InternalStats.java b/src/main/java/org/newrelic/nrjmx/v2/InternalStats.java
@@ -39,37 +39,41 @@ public InternalStats(long maxSize) {
      * Records a new InternalStat and returns it for attaching more data if required.
      *
      * @param statType String name of the stat.
-     *
      * @return InternalStat new registered stat to add data to it.
      */
     public InternalStat record(String statType) {
-        if (stats.size() >= maxSize) {
-            stats.remove(0);
-        }
+        synchronized (stats) {
+            if (stats.size() >= maxSize) {
+                stats.remove(0);
+            }
 
-        InternalStat stat = new InternalStat()
-                .setStartTimestamp(System.currentTimeMillis())
-                .setMilliseconds((double)System.nanoTime() / 1000000.0)
-                .setStatType(statType);
-        stats.add(stat);
-        return stat;
+            InternalStat stat = new InternalStat()
+                    .setStartTimestamp(System.currentTimeMillis())
+                    .setMilliseconds((double) System.nanoTime() / 1000000.0)
+                    .setStatType(statType);
+            stats.add(stat);
+            return stat;
+        }
     }
 
     /**
      * Returns all the collected stats and clear them.
-     **
+     *
+     *
      * @return List<InternalStat> returns all collected internal stats.
      */
     public List<InternalStat> getStats() {
-        List<InternalStat> stats = this.stats;
-        this.stats = new ArrayList<>();
-        return stats;
+        synchronized (this.stats) {
+            List<InternalStat> stats = new ArrayList<>(this.stats);
+            this.stats = new ArrayList<>();
+            return stats;
+        }
     }
 
     /**
      * Calculate the elapsed ms with .3f precision since the stat was recorded and attach it to the stat.
      */
     public static void setElapsedMs(InternalStat internalStat) {
-        internalStat.setMilliseconds((double)System.nanoTime() / 1000000.0 - internalStat.milliseconds);
+        internalStat.setMilliseconds((double) System.nanoTime() / 1000000.0 - internalStat.milliseconds);
     }
 }
diff --git a/src/main/java/org/newrelic/nrjmx/v2/JMXFetcher.java b/src/main/java/org/newrelic/nrjmx/v2/JMXFetcher.java
@@ -19,7 +19,6 @@
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.math.BigDecimal;
-import java.rmi.ConnectException;
 import java.text.DateFormat;
 import java.util.*;
 import java.util.concurrent.*;
@@ -134,6 +133,10 @@ public void disconnect(long timeoutMs) throws JMXError, JMXConnectionError {
      * @throws JMXConnectionError JMX connection related exception
      */
     public void disconnect() throws JMXConnectionError {
+        if (Thread.interrupted()) {
+            return;
+        }
+
         if (this.connector == null) {
             throw new JMXConnectionError()
                     .setMessage("cannot disconnect, connection to JMX endpoint is not established");
@@ -226,16 +229,18 @@ private Set<ObjectInstance> queryMBeans(ObjectName objectName) throws JMXConnect
             }
 
             return result;
-        } catch (ConnectException ce) {
+        } catch (JMXConnectionError je) {
+            throw je;
+        } catch (IOException io) {
             disconnect();
 
-            String message = String.format("can't connect to JMX server, error: '%s'", ce.getMessage());
+            String message = String.format("problem occurred when talking to the JMX server while querying mBeans, error: '%s'", io.getMessage());
             throw new JMXConnectionError(message);
-        } catch (IOException ioe) {
+        } catch (Exception e) {
             throw new JMXError()
                     .setMessage("can't get beans for query: " + objectName)
-                    .setCauseMessage(ioe.getMessage())
-                    .setStacktrace(getStackTrace(ioe));
+                    .setCauseMessage(e.getMessage())
+                    .setStacktrace(getStackTrace(e));
         } finally {
             if (internalStat != null) {
                 InternalStats.setElapsedMs(internalStat);
@@ -291,12 +296,14 @@ private List<String> getMBeanAttributeNames(ObjectName objectName) throws JMXCon
             if (internalStat != null) {
                 internalStat.setSuccessful(true);
             }
-        } catch (ConnectException ce) {
+        } catch (JMXConnectionError je) {
+            throw je;
+        } catch (IOException io) {
             disconnect();
 
-            String message = String.format("can't connect to JMX server, error: '%s'", ce.getMessage());
+            String message = String.format("problem occurred when talking to the JMX server while requesting mBean info, error: '%s'", io.getMessage());
             throw new JMXConnectionError(message);
-        } catch (InstanceNotFoundException | IntrospectionException | ReflectionException | IOException e) {
+        } catch (InstanceNotFoundException | IntrospectionException | ReflectionException e) {
             throw new JMXError()
                     .setMessage("can't find mBean: " + objectName)
                     .setCauseMessage(e.getMessage())
@@ -386,10 +393,12 @@ private void getMBeanAttributes(ObjectName objectName, List<String> attributes,
             if (internalStat != null) {
                 internalStat.setSuccessful(true);
             }
-        } catch (ConnectException ce) {
+        } catch (JMXConnectionError je) {
+            throw je;
+        } catch (IOException io) {
             disconnect();
 
-            String message = String.format("can't connect to JMX server, error: '%s'", ce.getMessage());
+            String message = String.format("problem occurred when talking to the JMX server while requesting attributes, error: '%s'", io.getMessage());
             throw new JMXConnectionError(message);
         } catch (Exception e) {
             throw new JMXError()
@@ -525,6 +534,7 @@ private ObjectName getObjectName(String mBeanName) throws JMXError {
      * @throws JMXConnectionError JMX connection related exception
      */
     private <T> T withTimeout(Future<T> future, long timeoutMs) throws JMXError, JMXConnectionError {
+
         try {
             if (timeoutMs <= 0) {
                 return future.get();
@@ -551,6 +561,8 @@ private <T> T withTimeout(Future<T> future, long timeoutMs) throws JMXError, JMX
             throw new JMXError()
                     .setMessage("failed to execute operation, error: " + e.getMessage())
                     .setStacktrace(getStackTrace(e));
+        } finally {
+            future.cancel(true);
         }
     }
 
@@ -637,7 +649,7 @@ public List<InternalStat> getInternalStats() throws JMXError {
      * @return MBeanServerConnection the connection to the JMX endpoint
      * @throws JMXConnectionError JMX connection related Exception
      */
-    private MBeanServerConnection getConnection() throws JMXConnectionError, JMXError {
+    private MBeanServerConnection getConnection() throws JMXConnectionError {
         if (jmxConfig == null) {
             throw new JMXConnectionError("failed to get connection to JMX server: configuration not provided");
         }
@@ -712,6 +724,7 @@ private static Map<String, Object> buildConnectionEnvConfig(JMXConfig jmxConfig)
             p.put("javax.net.ssl.trustStorePassword", jmxConfig.trustStorePassword);
             connectionEnv.put("com.sun.jndi.rmi.factory.socket", new SslRMIClientSocketFactory());
         }
+
         return connectionEnv;
     }