Update warning logs (#369)

* update how we log warnings

* add stack trace

* use name of class as type

* update casing

* update messages

* rename method

Author: molly-moen

org-code-javabuilder/lib/src/main/java/org/code/javabuilder/AWSSystemExitHelper.java

@@ -8,7 +8,7 @@
 import java.util.logging.Handler;
 import java.util.logging.Logger;
 import org.code.protocol.JavabuilderContext;
-import org.code.protocol.JavabuilderThrowableMessageUtils;
+import org.code.protocol.LoggerUtils;
 
 public class AWSSystemExitHelper implements SystemExitHelper {
   private final String connectionId;
@@ -42,7 +42,7 @@ private void cleanUpResources() {
       // Handle any other exceptions so that shut down proceeds normally. If this is an
       // IllegalStateException, it indicates that the connection was already shut down for
       // some reason.
-      Logger.getLogger(MAIN_LOGGER).warning(JavabuilderThrowableMessageUtils.getLoggingString(e));
+      LoggerUtils.logTrackingExceptionAsWarning(e);
     }
     // clean up log handler to avoid duplicate logs in future runs.
     Handler[] allHandlers = Logger.getLogger(MAIN_LOGGER).getHandlers();

org-code-javabuilder/lib/src/main/java/org/code/javabuilder/AssetFileStubber.java

@@ -1,8 +1,6 @@
 package org.code.javabuilder;
 
-import static org.code.protocol.LoggerNames.MAIN_LOGGER;
-
-import java.util.logging.Logger;
+import org.code.protocol.LoggerUtils;
 
 /**
  * Generates URLs for local, static asset files that can be used to stub asset file requests when
@@ -38,8 +36,9 @@ public String getStubAssetUrl(String filename) {
       return stubAudioUrl;
     }
 
-    Logger.getLogger(MAIN_LOGGER)
-        .warning(String.format("Unknown file %s. Cannot provide stubbed asset URL.", filename));
+    LoggerUtils.logWarning(
+        "Unknown Asset File",
+        String.format("Unknown file %s. Cannot provide stubbed asset URL.", filename));
     return null;
   }
 

org-code-javabuilder/lib/src/main/java/org/code/javabuilder/CodeExecutionManager.java

@@ -161,7 +161,7 @@ private void onPostExecute() {
       // If there was an issue clearing the temp directory, this may be because too many files are
       // open. Force the JVM to quit in order to release the resources for the next use of the
       // container. Temporarily logging the exception for investigation purposes.
-      LoggerUtils.logTrackingException(e);
+      LoggerUtils.logTrackingExceptionAsWarning(e);
       this.systemExitHelper.exit(TEMP_DIRECTORY_CLEANUP_ERROR_CODE);
     } finally {
       // Replace System in/out with original System in/out

org-code-javabuilder/lib/src/main/java/org/code/javabuilder/ExceptionHandler.java

@@ -1,8 +1,5 @@
 package org.code.javabuilder;
 
-import static org.code.protocol.LoggerNames.MAIN_LOGGER;
-
-import java.util.logging.Logger;
 import org.code.javabuilder.util.LambdaUtils;
 import org.code.protocol.*;
 
@@ -57,7 +54,7 @@ public void handle(Throwable e) {
     // Internal facing exceptions are caused by us (essentially an HTTP 5xx error), but don't affect
     // the user. Log only.
     if (e instanceof InternalFacingException || e instanceof InternalFacingRuntimeException) {
-      Logger.getLogger(MAIN_LOGGER).warning(((LoggableProtocol) e).getLoggingString());
+      LoggerUtils.logTrackingExceptionAsWarning(e);
       return;
     }
 

org-code-javabuilder/lib/src/main/java/org/code/javabuilder/LambdaRequestHandler.java

@@ -315,7 +315,7 @@ private void shutDown(
         // so these don't need to be reported to the user.
         final InternalFacingRuntimeException internal =
             new InternalFacingRuntimeException("Exception during shutdown", e);
-        Logger.getLogger(MAIN_LOGGER).warning(internal.getLoggingString());
+        LoggerUtils.logTrackingExceptionAsWarning(internal);
       }
     }
 
@@ -391,7 +391,7 @@ private void cleanUpAWSResources(String connectionId, AmazonApiGatewayManagement
       // Handle any other exceptions so that shut down proceeds normally. If this is an
       // IllegalStateException, it indicates that the connection was already shut down for
       // some reason.
-      Logger.getLogger(MAIN_LOGGER).warning(JavabuilderThrowableMessageUtils.getLoggingString(e));
+      LoggerUtils.logTrackingExceptionAsWarning(e);
     }
     // clean up log handler to avoid duplicate logs in future runs.
     Handler[] allHandlers = Logger.getLogger(MAIN_LOGGER).getHandlers();
@@ -408,9 +408,7 @@ private void verifyApiClient(String connectionId) {
     } catch (IllegalStateException e) {
       // This can occur if the api client has been shut down, which we have seen happen on occasion.
       // Recreate the api client in this case. Log a warning so we can track when this happens.
-      Logger.getLogger(MAIN_LOGGER)
-          .warning(
-              "Received illegal state exception when trying to talk to API Gateway. Recreating api client.");
+      LoggerUtils.logWarning("API Gateway Client Gone", e.getMessage());
       this.apiClient =
           AmazonApiGatewayManagementApiClientBuilder.standard()
               .withEndpointConfiguration(

org-code-javabuilder/lib/src/main/java/org/code/javabuilder/UserClassLoader.java

@@ -1,7 +1,5 @@
 package org.code.javabuilder;
 
-import static org.code.protocol.LoggerNames.MAIN_LOGGER;
-
 import java.lang.invoke.LambdaMetafactory;
 import java.lang.invoke.StringConcatFactory;
 import java.lang.reflect.InvocationTargetException;
@@ -11,9 +9,7 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
-import java.util.logging.Logger;
-import org.code.protocol.LoggerConstants;
-import org.json.JSONObject;
+import org.code.protocol.LoggerUtils;
 
 /**
  * Custom class loader for user-provided code. This class loader only allows certain classes to be
@@ -60,10 +56,7 @@ public Class<?> loadClass(String name) throws ClassNotFoundException {
 
     // Log that we are going to throw an exception. Log as a warning
     // as it is most likely user error, but we want to track it.
-    JSONObject eventData = new JSONObject();
-    eventData.put(LoggerConstants.TYPE, "invalidClass");
-    eventData.put(LoggerConstants.CLASS_NAME, name);
-    Logger.getLogger(MAIN_LOGGER).warning(eventData.toString());
+    LoggerUtils.logWarning("Invalid Class", name);
     throw new ClassNotFoundException(name);
   }
 

org-code-javabuilder/lib/src/main/java/org/code/javabuilder/UserCodeCompiler.java

@@ -1,12 +1,9 @@
 package org.code.javabuilder;
 
-import static org.code.protocol.LoggerNames.MAIN_LOGGER;
-
 import java.io.File;
 import java.io.IOException;
 import java.io.Reader;
 import java.util.*;
-import java.util.logging.Logger;
 import javax.tools.*;
 import javax.tools.JavaCompiler.CompilationTask;
 import org.code.javabuilder.util.JarUtils;
@@ -157,10 +154,10 @@ private String getCustomCompilerError(Diagnostic<? extends JavaFileObject> diagn
    */
   private String getCompilerError(Diagnostic<? extends JavaFileObject> diagnostic) {
     if (diagnostic.getSource() == null || diagnostic.getLineNumber() == Diagnostic.NOPOS) {
-      Logger.getLogger(MAIN_LOGGER)
-          .warning(
-              "Falling back to default compiler error, diagnostic source was null or line number was -1. Diagnostic error code is "
-                  + diagnostic.getCode());
+      LoggerUtils.logWarning(
+          "Unknown Compiler Error",
+          "Falling back to default compiler error, diagnostic source was null or line number was -1. Diagnostic error code is "
+              + diagnostic.getCode());
       return diagnostic.toString();
     }
     // Subtract 1 from the line number to account for our auto-import

org-code-javabuilder/lib/src/main/java/org/code/javabuilder/util/LambdaUtils.java

@@ -1,12 +1,10 @@
 package org.code.javabuilder.util;
 
 import static org.code.javabuilder.InternalFacingExceptionTypes.CONNECTION_TERMINATED;
-import static org.code.protocol.LoggerNames.MAIN_LOGGER;
 
-import java.util.logging.Logger;
 import org.code.javabuilder.InternalFacingRuntimeException;
 import org.code.protocol.ClientMessage;
-import org.code.protocol.JavabuilderThrowableMessageUtils;
+import org.code.protocol.LoggerUtils;
 import org.code.protocol.OutputAdapter;
 
 public final class LambdaUtils {
@@ -30,11 +28,11 @@ public static void safelySendMessage(
     } catch (InternalFacingRuntimeException e) {
       // Unless logOnLostConnection is true, only log for messages that aren't CONNECTION_TERMINATED
       if (logOnLostConnection || !e.getMessage().equals(CONNECTION_TERMINATED)) {
-        Logger.getLogger(MAIN_LOGGER).warning(e.getLoggingString());
+        LoggerUtils.logTrackingExceptionAsWarning(e);
       }
     } catch (Exception e) {
       // Catch any other exceptions here to prevent them from propagating.
-      Logger.getLogger(MAIN_LOGGER).warning(JavabuilderThrowableMessageUtils.getLoggingString(e));
+      LoggerUtils.logTrackingExceptionAsWarning(e);
     }
   }
 }

org-code-javabuilder/protocol/src/main/java/org/code/protocol/LoggerConstants.java

@@ -14,6 +14,8 @@ public class LoggerConstants {
   public static final String CHANNEL_ID = "channelId";
   public static final String MINI_APP_TYPE = "miniAppType";
   public static final String SESSION_METADATA = "sessionMetadata";
+  public static final String DETAIL = "detail";
+  public static final String STACK_TRACE = "stackTrace";
 
   // Constants associated with disk space metrics
   public static final String DIRECTORY = "directory";

org-code-javabuilder/protocol/src/main/java/org/code/protocol/LoggerUtils.java

@@ -3,6 +3,8 @@
 import static org.code.protocol.LoggerNames.MAIN_LOGGER;
 
 import java.io.File;
+import java.io.PrintWriter;
+import java.io.StringWriter;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.logging.Logger;
@@ -99,19 +101,31 @@ public static void logSevereException(Throwable e) {
    * Exceptions logged in this way are intended to be informative, but should be removed after a
    * period of time.
    */
-  public static void logTrackingException(Throwable e) {
+  public static void logTrackingExceptionAsWarning(Throwable e) {
     JSONObject eventData = new JSONObject();
+    eventData.put(LoggerConstants.TYPE, e.getClass().getName());
     eventData.put(LoggerConstants.EXCEPTION_MESSAGE, e.getMessage());
     if (e.getCause() != null) {
       eventData.put(LoggerConstants.CAUSE, e.getCause());
     }
+    StringWriter sw = new StringWriter();
+    PrintWriter pw = new PrintWriter(sw);
+    e.printStackTrace(pw);
+    eventData.put(LoggerConstants.STACK_TRACE, sw.toString());
     Logger.getLogger(MAIN_LOGGER).warning(eventData.toString());
   }
 
   public static void logInfo(String info) {
     Logger.getLogger(MAIN_LOGGER).info(info);
   }
 
+  public static void logWarning(String type, String detail) {
+    JSONObject eventData = new JSONObject();
+    eventData.put(LoggerConstants.TYPE, type);
+    eventData.put(LoggerConstants.DETAIL, detail);
+    Logger.getLogger(MAIN_LOGGER).warning(eventData.toString());
+  }
+
   private static void sendDiskSpaceLogs(String type) {
     File f = Paths.get(System.getProperty("java.io.tmpdir")).toFile();
     JSONObject eventData = new JSONObject();