@azure/core-amqp - npm Package Compare versions

Comparing version 1.0.0-preview.3 to 1.0.0-preview.4

changelog.md

@@ -0,2 +1,8 @@
+## 1.0.0-preview.4 - 7th October, 2019
+
+- Fixes bug where calling `sendRequest` from a `RequestResponseLink` mulitple
+  times in parallel would result in all but 1 calls being retried.
+
 ## 1.0.0-preview.3 - 9th September, 2019
+
 Updates types for better compatibility with TypeScript 3.6.x. (PR #4928)
@@ -7,6 +13,6 @@
 - Retry updates
-   - The properties on the `RetryConfig` interface have been refactored for ease of use. The new `RetryOptions` in it will hold configurations like the number of retries, delay between retries, per try timeout etc.
-   - Support for exponential retry has been added
-   - Support for cancellation has been added via an optional `AbortSignal` from the [@azure/abort-controller](https://www.npmjs.com/package/@azure/abort-controller) package.
-   - The `RequestResponseLink` class has been updated to not have retries anymore for the `sendRequest()` method. The caller of this method is expected to add the relevant retries.
+  - The properties on the `RetryConfig` interface have been refactored for ease of use. The new `RetryOptions` in it will hold configurations like the number of retries, delay between retries, per try timeout etc.
+  - Support for exponential retry has been added
+  - Support for cancellation has been added via an optional `AbortSignal` from the [@azure/abort-controller](https://www.npmjs.com/package/@azure/abort-controller) package.
+  - The `RequestResponseLink` class has been updated to not have retries anymore for the `sendRequest()` method. The caller of this method is expected to add the relevant retries.
 - All time related entites have been updated to use milli seconds as the unit of time for consistency.
@@ -16,3 +22,2 @@ - New error `InsufficientCreditError` is introduced for the scenario where [rhea](https://www.npmjs.com/package/rhea) is unable to send events due to its internal buffer being full. This is a transient error and so is treated as retryable.
 
-
 ## 1.0.0-preview.1 - 28th June, 2019
@@ -19,0 +24,0 @@

dist-esm/src/requestResponseLink.js

@@ -97,4 +97,2 @@ // Copyright (c) Microsoft Corporation. All rights reserved.
             const messageCallback = (context) => {
-                // remove the event listeners as they will be registered next time when someone makes a request.
-                this.receiver.removeListener(ReceiverEvents.message, messageCallback);
                 if (aborter) {
@@ -106,15 +104,19 @@ aborter.removeEventListener("abort", onAbort);
                 log.reqres("[%s] %s response: ", this.connection.id, request.to || "$management", context.message);
+                if (request.message_id !== responseCorrelationId &&
+                    request.correlation_id !== responseCorrelationId) {
+                    // do not remove message listener.
+                    // parallel requests listen on the same receiver, so continue waiting until respose that matches
+                    // request via correlationId is found.
+                    log.error("[%s] request-messageId | '%s' != '%s' | response-correlationId. " +
+                        "Hence dropping this response and waiting for the next one.", this.connection.id, request.message_id, responseCorrelationId);
+                    return;
+                }
+                // remove the event listeners as they will be registered next time when someone makes a request.
+                this.receiver.removeListener(ReceiverEvents.message, messageCallback);
                 if (info.statusCode > 199 && info.statusCode < 300) {
-                    if (request.message_id === responseCorrelationId ||
-                        request.correlation_id === responseCorrelationId) {
-                        if (!timeOver) {
-                            clearTimeout(waitTimer);
-                        }
-                        log.reqres("[%s] request-messageId | '%s' == '%s' | response-correlationId.", this.connection.id, request.message_id, responseCorrelationId);
-                        return resolve(context.message);
+                    if (!timeOver) {
+                        clearTimeout(waitTimer);
                     }
-                    else {
-                        log.error("[%s] request-messageId | '%s' != '%s' | response-correlationId. " +
-                            "Hence dropping this response and waiting for the next one.", this.connection.id, request.message_id, responseCorrelationId);
-                    }
+                    log.reqres("[%s] request-messageId | '%s' == '%s' | response-correlationId.", this.connection.id, request.message_id, responseCorrelationId);
+                    return resolve(context.message);
                 }
@@ -121,0 +123,0 @@ else {

package.json

 {
   "name": "@azure/core-amqp",
   "sdk-type": "client",
-  "version": "1.0.0-preview.3",
+  "version": "1.0.0-preview.4",
   "description": "Common library for amqp based azure sdks like @azure/event-hubs.",
@@ -35,4 +35,4 @@ "author": "Microsoft Corporation",
     "integration-test": "npm run integration-test:node && npm run integration-test:browser",
-    "lint:fix": "eslint -c ../../.eslintrc.json src test samples --ext .ts --fix",
-    "lint": "eslint -c ../../.eslintrc.json src test samples --ext .ts -f html -o core-amqp-lintReport.html || exit 0",
+    "lint:fix": "eslint package.json tsconfig.json src test samples --ext .ts --fix",
+    "lint": "eslint package.json tsconfig.json src test samples --ext .ts -f html -o core-amqp-lintReport.html || exit 0",
     "pack": "npm pack 2>&1",
@@ -77,3 +77,4 @@ "prebuild": "npm run clean",
   "devDependencies": {
-    "@azure/identity": "1.0.0-preview.3",
+    "@azure/eslint-plugin-azure-sdk": "^2.0.1",
+    "@azure/identity": "1.0.0-preview.4",
     "@types/chai": "^4.1.6",
@@ -80,0 +81,0 @@ "@types/chai-as-promised": "^7.1.0",

src/requestResponseLink.ts

@@ -146,4 +146,2 @@ // Copyright (c) Microsoft Corporation. All rights reserved.
       const messageCallback = (context: EventContext) => {
-        // remove the event listeners as they will be registered next time when someone makes a request.
-        this.receiver.removeListener(ReceiverEvents.message, messageCallback);
         if (aborter) {
@@ -160,26 +158,32 @@ aborter.removeEventListener("abort", onAbort);
         );
+        if (
+          request.message_id !== responseCorrelationId &&
+          request.correlation_id !== responseCorrelationId
+        ) {
+          // do not remove message listener.
+          // parallel requests listen on the same receiver, so continue waiting until respose that matches
+          // request via correlationId is found.
+          log.error(
+            "[%s] request-messageId | '%s' != '%s' | response-correlationId. " +
+              "Hence dropping this response and waiting for the next one.",
+            this.connection.id,
+            request.message_id,
+            responseCorrelationId
+          );
+          return;
+        }
+
+        // remove the event listeners as they will be registered next time when someone makes a request.
+        this.receiver.removeListener(ReceiverEvents.message, messageCallback);
         if (info.statusCode > 199 && info.statusCode < 300) {
-          if (
-            request.message_id === responseCorrelationId ||
-            request.correlation_id === responseCorrelationId
-          ) {
-            if (!timeOver) {
-              clearTimeout(waitTimer);
-            }
-            log.reqres(
-              "[%s] request-messageId | '%s' == '%s' | response-correlationId.",
-              this.connection.id,
-              request.message_id,
-              responseCorrelationId
-            );
-            return resolve(context.message);
-          } else {
-            log.error(
-              "[%s] request-messageId | '%s' != '%s' | response-correlationId. " +
-                "Hence dropping this response and waiting for the next one.",
-              this.connection.id,
-              request.message_id,
-              responseCorrelationId
-            );
+          if (!timeOver) {
+            clearTimeout(waitTimer);
           }
+          log.reqres(
+            "[%s] request-messageId | '%s' == '%s' | response-correlationId.",
+            this.connection.id,
+            request.message_id,
+            responseCorrelationId
+          );
+          return resolve(context.message);
         } else {
@@ -186,0 +190,0 @@ const condition =

No alert changes

Improved metrics

Total package byte prevSize

2185101

increased by0.07%

Lines of code

22401

increased by0.04%

Worsened metrics

Dev dependency count

49

increased by2.08%

No dependency changes