From a79e2bb6b9f276dc541f859000bc982b0b017329 Mon Sep 17 00:00:00 2001
From: Andrew Byrd <andrew@fastmail.net>
Date: Wed, 20 Dec 2023 13:32:33 +0800
Subject: [PATCH] report worker errors as much shorter stack trace

move filterStackTrace method into shared utility class
---
 .../components/eventbus/ErrorEvent.java       | 18 ++-----------
 .../analyst/cluster/RegionalWorkResult.java   | 10 ++++++--
 .../com/conveyal/r5/util/ExceptionUtils.java  | 25 +++++++++++++++++++
 3 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/src/main/java/com/conveyal/analysis/components/eventbus/ErrorEvent.java b/src/main/java/com/conveyal/analysis/components/eventbus/ErrorEvent.java
index 24dc542f1..a157a9d5b 100644
--- a/src/main/java/com/conveyal/analysis/components/eventbus/ErrorEvent.java
+++ b/src/main/java/com/conveyal/analysis/components/eventbus/ErrorEvent.java
@@ -2,6 +2,8 @@
 
 import com.conveyal.r5.util.ExceptionUtils;
 
+import static com.conveyal.r5.util.ExceptionUtils.filterStackTrace;
+
 /**
  * This Event is fired each time a Throwable (usually an Exception or Error) occurs on the backend. It can then be
  * recorded or tracked in various places - the console logs, Slack, etc. This could eventually be used for errors on
@@ -59,20 +61,4 @@ public String traceWithContext (boolean verbose) {
         return builder.toString();
     }
 
-    private static String filterStackTrace (String stackTrace) {
-        if (stackTrace == null) return null;
-        final String unknownFrame = "Unknown stack frame, probably optimized out by JVM.";
-        String error = stackTrace.lines().findFirst().get();
-        String frame = stackTrace.lines()
-                .map(String::strip)
-                .filter(s -> s.startsWith("at "))
-                .findFirst().orElse(unknownFrame);
-        String conveyalFrame = stackTrace.lines()
-                .map(String::strip)
-                .filter(s -> s.startsWith("at com.conveyal."))
-                .filter(s -> !frame.equals(s))
-                .findFirst().orElse("");
-        return String.join("\n", error, frame, conveyalFrame);
-    }
-
 }
diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/RegionalWorkResult.java b/src/main/java/com/conveyal/r5/analyst/cluster/RegionalWorkResult.java
index d15531c03..c606acc0f 100644
--- a/src/main/java/com/conveyal/r5/analyst/cluster/RegionalWorkResult.java
+++ b/src/main/java/com/conveyal/r5/analyst/cluster/RegionalWorkResult.java
@@ -69,11 +69,17 @@ public RegionalWorkResult(OneOriginResult result, RegionalTask task) {
         // TODO checkTravelTimeInvariants, checkAccessibilityInvariants to verify that values are monotonically increasing
     }
 
-    /** Constructor used when results for this origin are considered unusable due to an unhandled error. */
+    /**
+     * Constructor used when results for this origin are considered unusable due to an unhandled error. Besides the
+     * short-form exception, most result fields are left null. There is no information to communicate, and because
+     * errors are often produced faster than valid results, we don't want to flood the backend with unnecessarily
+     * voluminous error reports. The short-form exception message is used for a similar reason, to limit the total size
+     * of error messages.
+     */
     public RegionalWorkResult(Throwable t, RegionalTask task) {
         this.jobId = task.jobId;
         this.taskId = task.taskId;
-        this.error = ExceptionUtils.shortAndLongString(t);
+        this.error = ExceptionUtils.filterStackTrace(t);
     }
 
 }
diff --git a/src/main/java/com/conveyal/r5/util/ExceptionUtils.java b/src/main/java/com/conveyal/r5/util/ExceptionUtils.java
index 901651e85..fca364b2a 100644
--- a/src/main/java/com/conveyal/r5/util/ExceptionUtils.java
+++ b/src/main/java/com/conveyal/r5/util/ExceptionUtils.java
@@ -50,4 +50,29 @@ public static String shortAndLongString (Throwable throwable) {
         return shortCauseString(throwable) + "\n[detail follows]\n" + stackTraceString(throwable);
     }
 
+    /**
+     * Given a full stack trace string with one frame per line, keep only the exception name, the first stack frame,
+     * and all additional frames that come from Conveyal packages. This yields a much shorter stack trace that still
+     * shows where the exception was thrown and where the problem originates in our own code.
+     */
+    public static String filterStackTrace (String stackTrace) {
+        if (stackTrace == null) return null;
+        final String unknownFrame = "Unknown stack frame, probably optimized out by JVM.";
+        String error = stackTrace.lines().findFirst().get();
+        String frame = stackTrace.lines()
+                .map(String::strip)
+                .filter(s -> s.startsWith("at "))
+                .findFirst().orElse(unknownFrame);
+        String conveyalFrame = stackTrace.lines()
+                .map(String::strip)
+                .filter(s -> s.startsWith("at com.conveyal."))
+                .filter(s -> !frame.equals(s))
+                .findFirst().orElse("");
+        return String.join("\n", error, frame, conveyalFrame);
+    }
+
+    public static String filterStackTrace (Throwable throwable) {
+        return filterStackTrace(stackTraceString(throwable));
+    }
+
 }