Review feedback

deephaven · Jan 27, 2023 · c481992 · c481992
1 parent 846a42f
commit c481992
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 13 deletions.
diff --git a/web/client-api/src/main/java/io/deephaven/web/client/api/CoreClient.java b/web/client-api/src/main/java/io/deephaven/web/client/api/CoreClient.java
@@ -108,6 +108,9 @@ public Promise<Void> login(LoginCredentials credentials) {
                 }
             }
             return null;
+        }).catch_(ignore -> {
+            // Ignore this failure and suppress browser logging, we have a safe fallback
+            return Promise.resolve((Object) null);
         });
         return login;
     }

diff --git a/...-api/src/main/java/io/deephaven/web/client/api/barrage/stream/HandshakeStreamFactory.java b/...-api/src/main/java/io/deephaven/web/client/api/barrage/stream/HandshakeStreamFactory.java
@@ -30,6 +30,12 @@
  * are received.
  */
 public class HandshakeStreamFactory {
+
+    private static final String DATA_EVENT_LISTENER_NAME = "data";
+    private static final String END_EVENT_LISTENER_NAME = "end";
+    private static final String STATUS_EVENT_LISTENER_NAME = "status";
+    private static final String HEADERS_EVENT_LISTENER_NAME = "headers";
+
     public static BiDiStream<HandshakeRequest, HandshakeResponse> create(WorkerConnection connection) {
         return connection.<HandshakeRequest, HandshakeResponse>streamFactory().create(
                 metadata -> {
@@ -41,17 +47,18 @@ public static BiDiStream<HandshakeRequest, HandshakeResponse> create(WorkerConne
                     Client<HandshakeRequest, HandshakeResponse> client = Grpc.client(FlightService.Handshake,
                             (io.deephaven.javascript.proto.dhinternal.grpcweb.grpc.ClientRpcOptions) options);
                     client.onEnd((status, statusMessage, trailers) -> {
-                        listeners.get("status").forEach((item, index, arr) -> item.call(null,
+                        listeners.get(STATUS_EVENT_LISTENER_NAME).forEach((item, index, arr) -> item.call(null,
                                 ResponseStreamWrapper.Status.of(status, statusMessage, metadata)));
-                        listeners.get("end").forEach((item, index, arr) -> item.call(null,
+                        listeners.get(END_EVENT_LISTENER_NAME).forEach((item, index, arr) -> item.call(null,
                                 ResponseStreamWrapper.Status.of(status, statusMessage, metadata)));
                         listeners.clear();
                     });
                     client.onMessage(message -> {
-                        listeners.get("data").forEach((item, index, arr) -> item.call(null, message));
+                        listeners.get(DATA_EVENT_LISTENER_NAME).forEach((item, index, arr) -> item.call(null, message));
                     });
                     client.onHeaders(headers -> {
-                        listeners.get("headers").forEach((item, index, arr) -> item.call(null, headers));
+                        listeners.get(HEADERS_EVENT_LISTENER_NAME)
+                                .forEach((item, index, arr) -> item.call(null, headers));
                     });
                     client.start(metadata);
 
@@ -92,19 +99,21 @@ public BidirectionalStream<HandshakeRequest, HandshakeResponse> write(
                     props.setTransport(null);// ts doesnt expose these two, stick with defaults for now
                     props.setDebug(false);
                     props.setOnMessage(responseMessage -> {
-                        listeners.get("data").forEach((item, index, arr) -> item.call(null, responseMessage));
+                        listeners.get(DATA_EVENT_LISTENER_NAME)
+                                .forEach((item, index, arr) -> item.call(null, responseMessage));
                     });
                     props.setOnEnd((status, statusMessage, trailers) -> {
-                        listeners.get("status").forEach(
+                        listeners.get(STATUS_EVENT_LISTENER_NAME).forEach(
                                 (item, index, arr) -> item.call(null,
                                         ResponseStreamWrapper.Status.of(status, statusMessage, metadata)));
-                        listeners.get("end").forEach(
+                        listeners.get(END_EVENT_LISTENER_NAME).forEach(
                                 (item, index, arr) -> item.call(null,
                                         ResponseStreamWrapper.Status.of(status, statusMessage, metadata)));
                         listeners.clear();
                     });
                     props.setOnHeaders(headers -> {
-                        listeners.get("headers").forEach((item, index, arr) -> item.call(null, headers));
+                        listeners.get(HEADERS_EVENT_LISTENER_NAME)
+                                .forEach((item, index, arr) -> item.call(null, headers));
                     });
                     Request client = Grpc.invoke.onInvoke(BrowserFlightService.OpenHandshake, props);
 
@@ -129,10 +138,10 @@ public ResponseStream<HandshakeResponse> on(String type, Function handler) {
 
     private static Map<String, JsArray<Function>> listenerMap() {
         Map<String, JsArray<Function>> listeners = new HashMap<>();
-        listeners.put("data", new JsArray<>());
-        listeners.put("end", new JsArray<>());
-        listeners.put("status", new JsArray<>());
-        listeners.put("headers", new JsArray<>());
+        listeners.put(DATA_EVENT_LISTENER_NAME, new JsArray<>());
+        listeners.put(END_EVENT_LISTENER_NAME, new JsArray<>());
+        listeners.put(STATUS_EVENT_LISTENER_NAME, new JsArray<>());
+        listeners.put(HEADERS_EVENT_LISTENER_NAME, new JsArray<>());
         return listeners;
     }
 }
diff --git a/web/client-api/src/main/java/io/deephaven/web/client/ide/IdeConnection.java b/web/client-api/src/main/java/io/deephaven/web/client/ide/IdeConnection.java
@@ -13,6 +13,7 @@
 import io.deephaven.web.shared.fu.JsRunnable;
 import jsinterop.annotations.JsConstructor;
 import jsinterop.annotations.JsIgnore;
+import jsinterop.annotations.JsOptional;
 import jsinterop.annotations.JsType;
 import jsinterop.base.JsPropertyMap;
 
@@ -26,7 +27,7 @@ public class IdeConnection extends QueryConnectable<IdeConnection> {
     private final ConnectToken token = new ConnectToken();
 
     @JsConstructor
-    public IdeConnection(String serverUrl, Boolean fromJava) {
+    public IdeConnection(String serverUrl, @JsOptional Boolean fromJava) {
         this.serverUrl = serverUrl;
         deathListenerCleanup = JsRunnable.doNothing();