[core-amqp] adds support for idle timeout and additional logging (#6492)

* [core-amqp] adds logging around dns.resolve in checkNetworkConnection * [core-amqp] adds support for idle timeout check * updates pnpm lock file
AzureSDKAutomation · Dec 10, 2019 · 27d9cb1 · 27d9cb1
1 parent 94b5d23
commit 27d9cb1
Show file tree

Hide file tree

Showing 11 changed files with 392 additions and 330 deletions.
diff --git a/common/config/rush/pnpm-lock.yaml b/common/config/rush/pnpm-lock.yaml
diff --git a/sdk/core/core-amqp/changelog.md b/sdk/core/core-amqp/changelog.md
@@ -1,6 +1,11 @@
+## 1.0.0-preview.7 - TBD
+
+- Improved detection of when an established socket is no longer receiving data from the service.
+- Added logging around the network connectivity check.
+
 ## 1.0.0-preview.6 - 3rd December, 2019
 
-* Treat ETIMEOUT error from dns.resolve as network disconnected.
+- Treat ETIMEOUT error from dns.resolve as network disconnected.
 
 ## 1.0.0-preview.5 - 29th October, 2019
 

diff --git a/sdk/core/core-amqp/package.json b/sdk/core/core-amqp/package.json
@@ -1,14 +1,15 @@
 {
   "name": "@azure/core-amqp",
   "sdk-type": "client",
-  "version": "1.0.0-preview.6",
+  "version": "1.0.0-preview.7",
   "description": "Common library for amqp based azure sdks like @azure/event-hubs.",
   "author": "Microsoft Corporation",
   "license": "MIT",
   "main": "./dist/index.js",
   "module": "./dist-esm/src/index.js",
   "types": "./typings/src/index.d.ts",
   "browser": {
+    "./dist-esm/src/util/checkNetworkConnection.js": "./dist-esm/src/util/checkNetworkConnection.browser.js",
     "./dist/index.js": "./browser/index.js",
     "buffer": "buffer",
     "stream": "stream-browserify"
@@ -66,14 +67,13 @@
     "is-buffer": "^2.0.3",
     "jssha": "^2.3.1",
     "process": "^0.11.10",
+    "rhea": "^1.0.15",
+    "rhea-promise": "^1.0.0",
     "stream-browserify": "^2.0.2",
     "tslib": "^1.9.3",
     "url": "^0.11.0",
     "util": "^0.12.1"
   },
-  "peerDependencies": {
-    "rhea-promise": "^1.0.0"
-  },
   "devDependencies": {
     "@azure/eslint-plugin-azure-sdk": "^2.0.1",
     "@azure/identity": "^1.0.0",
@@ -107,8 +107,6 @@
     "nyc": "^14.0.0",
     "prettier": "^1.16.4",
     "puppeteer": "^2.0.0",
-    "rhea": "^1.0.4",
-    "rhea-promise": "^1.0.0",
     "rimraf": "^3.0.0",
     "rollup": "^1.16.3",
     "rollup-plugin-commonjs": "^10.0.0",

diff --git a/sdk/core/core-amqp/rollup.base.config.js b/sdk/core/core-amqp/rollup.base.config.js
@@ -14,7 +14,7 @@ import shim from "rollup-plugin-shim";
 import json from "@rollup/plugin-json";
 
 const pkg = require("./package.json");
-const depNames = Object.keys(pkg.dependencies).concat(Object.keys(pkg.peerDependencies));
+const depNames = Object.keys(pkg.dependencies);
 
 const input = "dist-esm/src/index.js";
 const production = process.env.NODE_ENV === "production";

diff --git a/sdk/core/core-amqp/src/ConnectionContextBase.ts b/sdk/core/core-amqp/src/ConnectionContextBase.ts
@@ -156,6 +156,7 @@ export module ConnectionContextBase {
         platform: `(${os.arch()}-${os.type()}-${os.release()})`,
         framework: `Node/${process.version}`
       },
+      idle_time_out: Constants.defaultConnectionIdleTimeoutInMs,
       operationTimeoutInSeconds: parameters.operationTimeoutInMs
         ? parameters.operationTimeoutInMs / 1000
         : undefined

diff --git a/sdk/core/core-amqp/src/retry.ts b/sdk/core/core-amqp/src/retry.ts
@@ -2,15 +2,15 @@
 // Licensed under the MIT License.
 
 import { translate, MessagingError } from "./errors";
-import { delay, isNode } from "./util/utils";
+import { delay } from "./util/utils";
 import * as log from "./log";
 import {
   defaultMaxRetries,
   defaultDelayBetweenOperationRetriesInMs,
   defaultMaxDelayForExponentialRetryInMs
 } from "./util/constants";
-import { resolve } from "dns";
 import { AbortSignalLike } from "@azure/abort-controller";
+import { checkNetworkConnection } from "./util/checkNetworkConnection";
 
 /**
  * Determines whether the object is a Delivery object.
@@ -139,23 +139,6 @@ function validateRetryConfig<T>(config: RetryConfig<T>): void {
   }
 }
 
-async function checkNetworkConnection(host: string): Promise<boolean> {
-  if (isNode) {
-    return new Promise((res) => {
-      resolve(host, function(err: any): void {
-        // List of possible DNS error codes: https://nodejs.org/dist/latest-v12.x/docs/api/dns.html#dns_error_codes
-        if (err && (err.code === "ECONNREFUSED" || err.code === "ETIMEOUT")) {
-          res(false);
-        } else {
-          res(true);
-        }
-      });
-    });
-  } else {
-    return window.navigator.onLine;
-  }
-}
-
 /**
  * Every operation is attempted at least once. Additional attempts are made if the previous attempt failed
  * with a retryable error. The number of additional attempts is governed by the `maxRetries` property provided

diff --git a/sdk/core/core-amqp/src/util/checkNetworkConnection.browser.ts b/sdk/core/core-amqp/src/util/checkNetworkConnection.browser.ts
@@ -0,0 +1,11 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+/**
+ * Checks whether a network connection is detected.
+ * @ignore
+ * @internal
+ */
+export function checkNetworkConnection(): Promise<boolean> {
+  return Promise.resolve(window.navigator.onLine);
+}
diff --git a/sdk/core/core-amqp/src/util/checkNetworkConnection.ts b/sdk/core/core-amqp/src/util/checkNetworkConnection.ts
@@ -0,0 +1,34 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+import { resolve, CONNREFUSED, TIMEOUT } from "dns";
+import { retry as logRetry } from "../log";
+
+/**
+ * Checks whether a network connection is detected.
+ * @ignore
+ * @internal
+ */
+export function checkNetworkConnection(host: string): Promise<boolean> {
+  return new Promise((res) => {
+    logRetry("Calling dns.resolve to determine network connection status.");
+    resolve(host, function(err: any): void {
+      if (err) {
+        logRetry(
+          "Error thrown from dns.resolve in network connection check: '%s', %O",
+          err.code || err.name,
+          err
+        );
+        // List of possible DNS error codes: https://nodejs.org/dist/latest-v12.x/docs/api/dns.html#dns_error_codes
+        // Only when dns.resolve returns an error we expect to see when the network is down, resolve as 'false'.
+        if (err.code === CONNREFUSED || err.code === TIMEOUT) {
+          return res(false);
+        }
+      } else {
+        logRetry("Successfully resolved host via dns.resolve in network connection check.");
+      }
+
+      return res(true);
+    });
+  });
+}
diff --git a/sdk/core/core-amqp/src/util/constants.ts b/sdk/core/core-amqp/src/util/constants.ts
@@ -51,6 +51,7 @@ export const senderError = "sender_error";
 export const sessionError = "session_error";
 export const connectionError = "connection_error";
 export const defaultOperationTimeoutInMs = 60000;
+export const defaultConnectionIdleTimeoutInMs = 60000;
 export const managementRequestKey = "managementRequest";
 export const negotiateCbsKey = "negotiateCbs";
 export const negotiateClaim = "negotiateClaim";

diff --git a/sdk/eventhub/testhub/package.json b/sdk/eventhub/testhub/package.json
@@ -41,7 +41,7 @@
     "async-lock": "^1.1.3",
     "death": "^1.1.0",
     "debug": "^4.1.1",
-    "rhea": "^1.0.4",
+    "rhea": "^1.0.15",
     "rimraf": "^3.0.0",
     "tslib": "^1.9.3",
     "typescript": "~3.6.4",

diff --git a/sdk/servicebus/service-bus/package.json b/sdk/servicebus/service-bus/package.json
@@ -79,7 +79,7 @@
     "is-buffer": "^2.0.3",
     "long": "^4.0.0",
     "process": "^0.11.10",
-    "rhea": "^1.0.4",
+    "rhea": "^1.0.15",
     "rhea-promise": "^0.1.15",
     "tslib": "^1.9.3"
   },