From 8c1628f4e68f4aff5f95244d054ee08b6f951b08 Mon Sep 17 00:00:00 2001
From: Nathaniel Bauernfeind <natebauernfeind@deephaven.io>
Date: Tue, 17 Oct 2023 13:44:57 -0600
Subject: [PATCH] Ryan's feedback v3

---
 .../barrage/table/BarrageTable.java           | 23 ++++++++++++++-----
 .../client/impl/BarrageSnapshotImpl.java      |  4 ++--
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/extensions/barrage/src/main/java/io/deephaven/extensions/barrage/table/BarrageTable.java b/extensions/barrage/src/main/java/io/deephaven/extensions/barrage/table/BarrageTable.java
index 2ddf1831837..745cbf6ee4a 100644
--- a/extensions/barrage/src/main/java/io/deephaven/extensions/barrage/table/BarrageTable.java
+++ b/extensions/barrage/src/main/java/io/deephaven/extensions/barrage/table/BarrageTable.java
@@ -147,9 +147,10 @@ public interface ViewportChangedCallback {
 
     private final SourceRefresher refresher;
 
-    // Used to notify a listener that the viewport has changed. This is typically used by the caller to know when the
-    // server
-    // has acknowledged a viewport change request.
+    /**
+     * Used to notify a listener that the viewport has changed. This is typically used by the caller to know when the
+     * server has acknowledged a viewport change request.
+     */
     @Nullable
     private ViewportChangedCallback viewportChangedCallback;
 
@@ -283,9 +284,19 @@ protected void instrumentedRefresh() {
                 final long startTm = System.nanoTime();
                 realRefresh();
                 recordMetric(stats -> stats.refresh, System.nanoTime() - startTm);
-            } catch (Exception e) {
-                beginLog(LogLevel.ERROR).append(": Failure during BarrageTable run: ").append(e).endl();
-                notifyListenersOnError(e, null);
+            } catch (Throwable err) {
+                beginLog(LogLevel.ERROR).append(": Failure during BarrageTable instrumentedRefresh: ")
+                        .append(err).endl();
+                notifyListenersOnError(err, null);
+
+                if (viewportChangedCallback != null) {
+                    viewportChangedCallback.onError(err);
+                    viewportChangedCallback = null;
+                }
+                if (err instanceof Error) {
+                    // rethrow if this was an error (which should not be swallowed)
+                    throw err;
+                }
             }
         }
     }
diff --git a/java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSnapshotImpl.java b/java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSnapshotImpl.java
index ef39dd5d37c..cd80dae4d99 100644
--- a/java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSnapshotImpl.java
+++ b/java-client/barrage/src/main/java/io/deephaven/client/impl/BarrageSnapshotImpl.java
@@ -245,12 +245,12 @@ public BarrageTable blockUntilComplete() throws InterruptedException {
         }
 
         if (holdingUpdateGraphLock) {
-            while (!checkIfCompletedOrThrow()) {
+            while (!checkIfCompleteOrThrow()) {
                 completedCondition.await();
             }
         } else {
             synchronized (this) {
-                while (!checkIfCompletedOrThrow()) {
+                while (!checkIfCompleteOrThrow()) {
                     wait(); // BarrageSnapshotImpl lock
                 }
             }