From 1ed1c9d3b9c6a88322cfac489215fb3b850d7c29 Mon Sep 17 00:00:00 2001
From: Colin Alworth <colinalworth@deephaven.io>
Date: Wed, 11 Dec 2024 21:11:39 -0600
Subject: [PATCH] fix: reapply RST_STREAM(cancel) workaround, and disable
 grpc-timeout (#6478)

Changes from #6420 were supposed to be reverted in #6424, but apparently
git interpreted the branch's relationship with main differently than I
did.

This patch reapplies #6401, with an additional change to avoid noisy
stack traces from grpc clients that set `grpc-timeout`. At this time,
Deephaven doesn't handle timeouts nor do any default clients set
timeouts, so the impact here should be minimal.

Fixes #5996
See #6400
---
 .../servlet/jakarta/AsyncServletOutputStreamWriter.java   | 8 +++++++-
 .../main/java/io/grpc/servlet/jakarta/ServletAdapter.java | 5 ++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/grpc-java/grpc-servlet-jakarta/src/main/java/io/grpc/servlet/jakarta/AsyncServletOutputStreamWriter.java b/grpc-java/grpc-servlet-jakarta/src/main/java/io/grpc/servlet/jakarta/AsyncServletOutputStreamWriter.java
index 8b2c1da5412..9384be40faf 100644
--- a/grpc-java/grpc-servlet-jakarta/src/main/java/io/grpc/servlet/jakarta/AsyncServletOutputStreamWriter.java
+++ b/grpc-java/grpc-servlet-jakarta/src/main/java/io/grpc/servlet/jakarta/AsyncServletOutputStreamWriter.java
@@ -121,9 +121,15 @@ public boolean isFinestEnabled() {
             transportState.runOnTransportThread(
                     () -> {
                         transportState.complete();
-                        asyncContext.complete();
+                        // asyncContext.complete();
                         log.fine("call completed");
                     });
+            // Jetty specific fix: When AsyncContext.complete() is called, Jetty sends a RST_STREAM with
+            // "cancel" error to the client, while other containers send "no error" in this case. Calling
+            // close() instead on the output stream still sends the RST_STREAM, but with "no error". Note
+            // that this does the opposite in at least Tomcat, so we're not going to upstream this change.
+            // See https://github.com/deephaven/deephaven-core/issues/6400
+            outputStream.close();
         };
         this.isReady = () -> outputStream.isReady();
     }
diff --git a/grpc-java/grpc-servlet-jakarta/src/main/java/io/grpc/servlet/jakarta/ServletAdapter.java b/grpc-java/grpc-servlet-jakarta/src/main/java/io/grpc/servlet/jakarta/ServletAdapter.java
index 5be92f0989e..379ee668a20 100644
--- a/grpc-java/grpc-servlet-jakarta/src/main/java/io/grpc/servlet/jakarta/ServletAdapter.java
+++ b/grpc-java/grpc-servlet-jakarta/src/main/java/io/grpc/servlet/jakarta/ServletAdapter.java
@@ -143,7 +143,10 @@ public void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOEx
             logger.log(FINEST, "[{0}] headers: {1}", new Object[] {logId, headers});
         }
 
-        Long timeoutNanos = headers.get(TIMEOUT_KEY);
+        // Always ignore grpc-timeout at this time, as the servlet timeout isn't being reset
+        // when the output stream is closed. See https://github.com/deephaven/deephaven-core/issues/6400
+        // for more information.
+        Long timeoutNanos = null; // headers.get(TIMEOUT_KEY);
         if (timeoutNanos == null) {
             timeoutNanos = 0L;
         }