Fix web proxy (hiero-ledger#1395)

* web proxy issues tracking Signed-off-by: ochikov <[email protected]> * added nodeAccountIds unlocking Signed-off-by: Petar Tonev <[email protected]> * unhealthy nodes now are handled correctly Signed-off-by: Petar Tonev <[email protected]> * added additional proxies urls and after task build refactoring Signed-off-by: Petar Tonev <[email protected]> * add urls to a new client constants file Signed-off-by: Petar Tonev <[email protected]> * fix integration tests Signed-off-by: Petar Tonev <[email protected]> * handle some corner cases Signed-off-by: Petar Tonev <[email protected]> Signed-off-by: ochikov <[email protected]> Signed-off-by: Petar Tonev <[email protected]> Co-authored-by: ochikov <[email protected]>
monostarter · Jan 16, 2023 · 878e826 · 878e826
1 parent 089e469
commit 878e826
Show file tree

Hide file tree

Showing 10 changed files with 232 additions and 52 deletions.
diff --git a/src/Executable.js b/src/Executable.js
@@ -23,6 +23,7 @@ import GrpcStatus from "./grpc/GrpcStatus.js";
 import List from "./transaction/List.js";
 import Logger from "js-logger";
 import * as hex from "./encoding/hex.js";
+import HttpError from "./http/HttpError.js";
 
 /**
  * @typedef {import("./account/AccountId.js").default} AccountId
@@ -440,16 +441,22 @@ export default class Executable {
      * Unlike `shouldRetry` this method does in fact still return a boolean
      *
      * @protected
-     * @param {GrpcServiceError} error
+     * @param {Error} error
      * @returns {boolean}
      */
     _shouldRetryExceptionally(error) {
-        return (
-            error.status._code === GrpcStatus.Unavailable._code ||
-            error.status._code === GrpcStatus.ResourceExhausted._code ||
-            (error.status._code === GrpcStatus.Internal._code &&
-                RST_STREAM.test(error.message))
-        );
+        if (error instanceof GrpcServiceError) {
+            return (
+                error.status._code === GrpcStatus.Unavailable._code ||
+                error.status._code === GrpcStatus.ResourceExhausted._code ||
+                (error.status._code === GrpcStatus.Internal._code &&
+                    RST_STREAM.test(error.message))
+            );
+        } else {
+            // if we get to the 'else' statement, the 'error' is instanceof 'HttpError'
+            // and in this case, we have to retry always
+            return true;
+        }
     }
 
     /**
@@ -631,11 +638,12 @@ export default class Executable {
                 // Save the error in case we retry
                 persistentError = error;
                 Logger.debug(
-                    `[${logId}] received gRPC error ${JSON.stringify(error)}`
+                    `[${logId}] received error ${JSON.stringify(error)}`
                 );
 
                 if (
-                    error instanceof GrpcServiceError &&
+                    (error instanceof GrpcServiceError ||
+                        error instanceof HttpError) &&
                     this._shouldRetryExceptionally(error) &&
                     attempt <= maxAttempts
                 ) {

diff --git a/src/channel/NativeChannel.js b/src/channel/NativeChannel.js
@@ -20,6 +20,8 @@
 
 import Channel, { encodeRequest, decodeUnaryResponse } from "./Channel.js";
 import * as base64 from "../encoding/base64.native.js";
+import HttpError from "../http/HttpError.js";
+import HttpStatus from "../http/HttpStatus.js";
 
 export default class NativeChannel extends Channel {
     /**
@@ -71,6 +73,13 @@ export default class NativeChannel extends Channel {
                     }
                 );
 
+                if (!response.ok) {
+                    const error = new HttpError(
+                        HttpStatus._fromValue(response.status)
+                    );
+                    callback(error, null);
+                }
+
                 const blob = await response.blob();
 
                 /** @type {string} */

diff --git a/src/channel/WebChannel.js b/src/channel/WebChannel.js
@@ -20,6 +20,8 @@
 
 import GrpcServiceError from "../grpc/GrpcServiceError.js";
 import GrpcStatus from "../grpc/GrpcStatus.js";
+import HttpError from "../http/HttpError.js";
+import HttpStatus from "../http/HttpStatus.js";
 import Channel, { encodeRequest, decodeUnaryResponse } from "./Channel.js";
 
 export default class WebChannel extends Channel {
@@ -67,6 +69,13 @@ export default class WebChannel extends Channel {
                     }
                 );
 
+                if (!response.ok) {
+                    const error = new HttpError(
+                        HttpStatus._fromValue(response.status)
+                    );
+                    callback(error, null);
+                }
+
                 // Check headers for gRPC errors
                 const grpcStatus = response.headers.get("grpc-status");
                 const grpcMessage = response.headers.get("grpc-message");

diff --git a/src/client/ManagedNetwork.js b/src/client/ManagedNetwork.js
@@ -69,6 +69,14 @@ export default class ManagedNetwork {
          * @type {NetworkNodeT[]}
          */
         this._healthyNodes = [];
+
+        /**
+         * Count of unhealthy nodes.
+         *
+         * @protected
+         * @type {number}
+         */
+        this._unhealthyNodesCount = 0;
 
         /** @type {(address: string, cert?: string) => ChannelT} */
         this._createNetworkChannel = createNetworkChannel;
@@ -276,16 +284,17 @@ export default class ManagedNetwork {
         // `this._healthyNodes.length` times. This can result in a shorter
         // list than `count`, but that is much better than running forever
         for (let i = 0; i < this._healthyNodes.length; i++) {
-            if (nodes.length == count) {
+            if (nodes.length == count - this._unhealthyNodesCount) {
                 break;
             }
 
             // Get a random node
-            const node = this.getNode();
-
+            let node = this.getNode();
             if (!keys.has(node.getKey())) {
                 keys.add(node.getKey());
                 nodes.push(node);
+            } else {
+                i--;
             }
         }
 
@@ -485,8 +494,7 @@ export default class ManagedNetwork {
      */
     getNode(key) {
         this._readmitNodes();
-
-        if (key != null) {
+        if (key != null && key != undefined) {
             return /** @type {NetworkNodeT[]} */ (
                 this._network.get(key.toString())
             )[0];
@@ -510,6 +518,7 @@ export default class ManagedNetwork {
         for (let i = 0; i < this._healthyNodes.length; i++) {
             if (this._healthyNodes[i] == node) {
                 this._healthyNodes.splice(i, 1);
+                this._unhealthyNodesCount++;
             }
         }
     }

diff --git a/src/client/NativeClient.js b/src/client/NativeClient.js
@@ -20,8 +20,14 @@
 
 import Client from "./Client.js";
 import NativeChannel from "../channel/NativeChannel.js";
+// eslint-disable-next-line @typescript-eslint/no-unused-vars
 import AccountId from "../account/AccountId.js";
 import LedgerId from "../LedgerId.js";
+import {
+    MAINNET,
+    NATIVE_TESTNET,
+    NATIVE_PREVIEWNET,
+} from "../constants/ClientConstants.js";
 
 /**
  * @typedef {import("./Client.js").ClientConfiguration} ClientConfiguration
@@ -48,17 +54,9 @@ export const Network = {
         }
     },
 
-    MAINNET: {
-        "https://grpc-web.myhbarwallet.com:443": new AccountId(3),
-    },
-
-    TESTNET: {
-        "https://grpc-web.testnet.myhbarwallet.com:443": new AccountId(3),
-    },
-
-    PREVIEWNET: {
-        "https://grpc-web.previewnet.myhbarwallet.com:443": new AccountId(3),
-    },
+    MAINNET: MAINNET,
+    TESTNET: NATIVE_TESTNET,
+    PREVIEWNET: NATIVE_PREVIEWNET,
 };
 
 /**

diff --git a/src/client/Network.js b/src/client/Network.js
@@ -249,8 +249,12 @@ export default class Network extends ManagedNetwork {
         if (this._maxNodesPerTransaction > 0) {
             return this._maxNodesPerTransaction;
         }
-
-        return (this._nodes.length + 3 - 1) / 3;
+        // ultimately it does not matter if we round up or down
+        // if we round up, we will eventually take one more healthy node for execution
+        // and we would hit the 'nodes.length == count' check in _getNumberOfMostHealthyNodes() less often
+        return (this._nodes.length <= 9)
+            ? this._nodes.length
+            : Math.floor((this._nodes.length + 3 - 1) / 3)
     }
 
     /**

diff --git a/src/client/WebClient.js b/src/client/WebClient.js
@@ -20,8 +20,14 @@
 
 import Client from "./Client.js";
 import WebChannel from "../channel/WebChannel.js";
+// eslint-disable-next-line @typescript-eslint/no-unused-vars
 import AccountId from "../account/AccountId.js";
 import LedgerId from "../LedgerId.js";
+import {
+    MAINNET,
+    WEB_TESTNET,
+    WEB_PREVIEWNET,
+} from "../constants/ClientConstants.js";
 
 /**
  * @typedef {import("./Client.js").ClientConfiguration} ClientConfiguration
@@ -48,30 +54,9 @@ export const Network = {
         }
     },
 
-    MAINNET: {
-        "https://grpc-web.myhbarwallet.com:443": new AccountId(3),
-        "https://node01-00-grpc.swirlds.com:443": new AccountId(4),
-    },
-
-    TESTNET: {
-        "https://testnet-node00-00-grpc.hedera.com:443": new AccountId(3),
-        "https://testnet-node01-00-grpc.hedera.com:443": new AccountId(4),
-        "https://testnet-node02-00-grpc.hedera.com:443": new AccountId(5),
-        "https://testnet-node03-00-grpc.hedera.com:443": new AccountId(6),
-        "https://testnet-node04-00-grpc.hedera.com:443": new AccountId(7),
-        "https://testnet-node05-00-grpc.hedera.com:443": new AccountId(8),
-        "https://testnet-node06-00-grpc.hedera.com:443": new AccountId(9),
-    },
-
-    PREVIEWNET: {
-        "https://previewnet-node00-00-grpc.hedera.com:443": new AccountId(3),
-        "https://previewnet-node01-00-grpc.hedera.com:443": new AccountId(4),
-        "https://previewnet-node02-00-grpc.hedera.com:443": new AccountId(5),
-        "https://previewnet-node03-00-grpc.hedera.com:443": new AccountId(6),
-        "https://previewnet-node04-00-grpc.hedera.com:443": new AccountId(7),
-        "https://previewnet-node05-00-grpc.hedera.com:443": new AccountId(8),
-        "https://previewnet-node06-00-grpc.hedera.com:443": new AccountId(9),
-    },
+    MAINNET: MAINNET,
+    TESTNET: WEB_TESTNET,
+    PREVIEWNET: WEB_PREVIEWNET,
 };
 
 /**
@@ -83,7 +68,6 @@ export default class WebClient extends Client {
      */
     constructor(props) {
         super(props);
-
         if (props != null) {
             if (typeof props.network === "string") {
                 switch (props.network) {

diff --git a/src/constants/ClientConstants.js b/src/constants/ClientConstants.js
@@ -0,0 +1,59 @@
+import AccountId from "../account/AccountId.js";
+
+// MAINNET node proxies are the same for both 'WebClient' and 'NativeClient'
+export const MAINNET = {
+    "https://grpc-web.myhbarwallet.com:443": new AccountId(3),
+    "https://node01-00-grpc.swirlds.com:443": new AccountId(4),
+    "https://node02.swirldslabs.com:443": new AccountId(5),
+    "https://node03.swirldslabs.com:443": new AccountId(6),
+    "https://node04.swirldslabs.com:443": new AccountId(7),
+    "https://node05.swirldslabs.com:443": new AccountId(8),
+    "https://node06.swirldslabs.com:443": new AccountId(9),
+    "https://node07.swirldslabs.com:443": new AccountId(10),
+    "https://node08.swirldslabs.com:443": new AccountId(11),
+    "https://node09.swirldslabs.com:443": new AccountId(12),
+    "https://node10.swirldslabs.com:443": new AccountId(13),
+    "https://node11.swirldslabs.com:443": new AccountId(14),
+    "https://node12.swirldslabs.com:443": new AccountId(15),
+    "https://node13.swirldslabs.com:443": new AccountId(16),
+    "https://node14.swirldslabs.com:443": new AccountId(17),
+    "https://node16.swirldslabs.com:443": new AccountId(19),
+    "https://node17.swirldslabs.com:443": new AccountId(20),
+    "https://node18.swirldslabs.com:443": new AccountId(21),
+    "https://node19.swirldslabs.com:443": new AccountId(22),
+    "https://node20.swirldslabs.com:443": new AccountId(23),
+    "https://node21.swirldslabs.com:443": new AccountId(24),
+    "https://node22.swirldslabs.com:443": new AccountId(25),
+    "https://node23.swirldslabs.com:443": new AccountId(26),
+    "https://node24.swirldslabs.com:443": new AccountId(27),
+    "https://node25.swirldslabs.com:443": new AccountId(28),
+    "https://node26.swirldslabs.com:443": new AccountId(29),
+};
+
+export const WEB_TESTNET = {
+    "https://testnet-node00-00-grpc.hedera.com:443": new AccountId(3),
+    "https://testnet-node01-00-grpc.hedera.com:443": new AccountId(4),
+    "https://testnet-node02-00-grpc.hedera.com:443": new AccountId(5),
+    "https://testnet-node03-00-grpc.hedera.com:443": new AccountId(6),
+    "https://testnet-node04-00-grpc.hedera.com:443": new AccountId(7),
+    "https://testnet-node05-00-grpc.hedera.com:443": new AccountId(8),
+    "https://testnet-node06-00-grpc.hedera.com:443": new AccountId(9),
+};
+
+export const WEB_PREVIEWNET = {
+    "https://previewnet-node00-00-grpc.hedera.com:443": new AccountId(3),
+    "https://previewnet-node01-00-grpc.hedera.com:443": new AccountId(4),
+    "https://previewnet-node02-00-grpc.hedera.com:443": new AccountId(5),
+    "https://previewnet-node03-00-grpc.hedera.com:443": new AccountId(6),
+    "https://previewnet-node04-00-grpc.hedera.com:443": new AccountId(7),
+    "https://previewnet-node05-00-grpc.hedera.com:443": new AccountId(8),
+    "https://previewnet-node06-00-grpc.hedera.com:443": new AccountId(9),
+};
+
+export const NATIVE_TESTNET = {
+    "https://grpc-web.testnet.myhbarwallet.com:443": new AccountId(3),
+};
+
+export const NATIVE_PREVIEWNET = {
+    "https://grpc-web.previewnet.myhbarwallet.com:443": new AccountId(3),
+};
diff --git a/src/http/HttpError.js b/src/http/HttpError.js
@@ -0,0 +1,44 @@
+/*-
+ * ‌
+ * Hedera JavaScript SDK
+ * 
+ * Copyright (C) 2020 - 2022 Hedera Hashgraph, LLC
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * ‍
+ */
+// eslint-disable-next-line @typescript-eslint/no-unused-vars
+import HttpStatus from "./HttpStatus.js";
+
+/**
+ * Describes how the http request failed.
+ */
+export default class HttpError extends Error {
+    /**
+     * @param {HttpStatus} status
+     */
+    constructor(status) {
+        super(`failed with error code: ${status.toString()}`);
+
+        /**
+         * @readonly
+         */
+        this.status = status;
+
+        this.name = "HttpError";
+
+        if (typeof Error.captureStackTrace !== "undefined") {
+            Error.captureStackTrace(this, HttpError);
+        }
+    }
+}