@grpc/grpc-js - npm Package Compare versions

Comparing version 0.7.3 to 0.7.4

build/src/resolver-dns.js

@@ -59,20 +59,3 @@ "use strict";
 const DEFAULT_PORT = '443';
-/**
- * Get a promise that always resolves with either the result of the function
- * or the error if it failed.
- * @param fn
- */
-function resolvePromisify(fn) {
-    return arg => new Promise((resolve, reject) => {
-        fn(arg, (error, result) => {
-            if (error) {
-                resolve(error);
-            }
-            else {
-                resolve(result);
-            }
-        });
-    });
-}
-const resolveTxtPromise = resolvePromisify(dns.resolveTxt);
+const resolveTxtPromise = util.promisify(dns.resolveTxt);
 const dnsLookupPromise = util.promisify(dns.lookup);
@@ -126,5 +109,7 @@ /**
         this.listener = listener;
-        /* The promise results here contain, in order, the A record, the AAAA record,
-         * and either the TXT record or an error if TXT resolution failed */
-        this.pendingResultPromise = null;
+        this.pendingLookupPromise = null;
+        this.pendingTxtPromise = null;
+        this.latestLookupResult = null;
+        this.latestServiceConfig = null;
+        this.latestServiceConfigError = null;
         trace('Resolver constructed for target ' + target);
@@ -155,3 +140,3 @@ this.ipResult = parseIP(target);
      * If the target is an IP address, just provide that address as a result.
-     * Otherwise, initiate A, AAAA, and TXT
+     * Otherwise, initiate A, AAAA, and TXT lookups
      */
@@ -167,2 +152,9 @@ startResolution() {
         if (this.dnsHostname !== null) {
+            /* We clear out latestLookupResult here to ensure that it contains the
+             * latest result since the last time we started resolving. That way, the
+             * TXT resolution handler can use it, but only if it finishes second. We
+             * don't clear out any previous service config results because it's
+             * better to use a service config that's slightly out of date than to
+             * revert to an effectively blank one. */
+            this.latestLookupResult = null;
             const hostname = this.dnsHostname;
@@ -173,15 +165,10 @@ /* We lookup both address families here and then split them up later
              * error is indistinguishable from other kinds of errors */
-            const addressResult = dnsLookupPromise(hostname, { all: true });
-            /* We handle the TXT query promise differently than the others because
-             * the name resolution attempt as a whole is a success even if the TXT
-             * lookup fails */
-            const txtResult = resolveTxtPromise(hostname);
-            this.pendingResultPromise = Promise.all([addressResult, txtResult]);
-            this.pendingResultPromise.then(([addressList, txtRecord]) => {
-                this.pendingResultPromise = null;
+            this.pendingLookupPromise = dnsLookupPromise(hostname, { all: true });
+            this.pendingLookupPromise.then(addressList => {
+                this.pendingLookupPromise = null;
                 const ip4Addresses = addressList.filter(addr => addr.family === 4);
                 const ip6Addresses = addressList.filter(addr => addr.family === 6);
-                const allAddresses = mergeArrays(ip4Addresses, ip6Addresses).map(addr => ({ host: addr.address, port: +this.port }));
+                this.latestLookupResult = mergeArrays(ip6Addresses, ip4Addresses).map(addr => ({ host: addr.address, port: +this.port }));
                 const allAddressesString = '[' +
-                    allAddresses.map(addr => addr.host + ':' + addr.port).join(',') +
+                    this.latestLookupResult.map(addr => addr.host + ':' + addr.port).join(',') +
                     ']';
@@ -192,21 +179,33 @@ trace('Resolved addresses for target ' +
                     allAddressesString);
-                if (allAddresses.length === 0) {
+                if (this.latestLookupResult.length === 0) {
                     this.listener.onError(this.defaultResolutionError);
                     return;
                 }
-                let serviceConfig = null;
-                let serviceConfigError = null;
-                if (txtRecord instanceof Error) {
-                    serviceConfigError = {
-                        code: constants_1.Status.UNAVAILABLE,
-                        details: 'TXT query failed',
-                        metadata: new metadata_1.Metadata(),
-                    };
-                }
-                else {
+                /* If the TXT lookup has not yet finished, both of the last two
+                 * arguments will be null, which is the equivalent of getting an
+                 * empty TXT response. When the TXT lookup does finish, its handler
+                 * can update the service config by using the same address list */
+                this.listener.onSuccessfulResolution(this.latestLookupResult, this.latestServiceConfig, this.latestServiceConfigError);
+            }, err => {
+                trace('Resolution error for target ' +
+                    this.target +
+                    ': ' +
+                    err.message);
+                this.pendingLookupPromise = null;
+                this.listener.onError(this.defaultResolutionError);
+            });
+            /* If there already is a still-pending TXT resolution, we can just use
+             * that result when it comes in */
+            if (this.pendingTxtPromise === null) {
+                /* We handle the TXT query promise differently than the others because
+                 * the name resolution attempt as a whole is a success even if the TXT
+                 * lookup fails */
+                this.pendingTxtPromise = resolveTxtPromise(hostname);
+                this.pendingTxtPromise.then(txtRecord => {
+                    this.pendingTxtPromise = null;
                     try {
-                        serviceConfig = service_config_1.extractAndSelectServiceConfig(txtRecord, this.percentage);
+                        this.latestServiceConfig = service_config_1.extractAndSelectServiceConfig(txtRecord, this.percentage);
                     }
                     catch (err) {
-                        serviceConfigError = {
+                        this.latestServiceConfigError = {
                             code: constants_1.Status.UNAVAILABLE,
@@ -217,12 +216,20 @@ details: 'Parsing service config failed',
                     }
-                }
-                this.listener.onSuccessfulResolution(allAddresses, serviceConfig, serviceConfigError);
-            }, err => {
-                trace('Resolution error for target ' +
-                    this.target +
-                    ': ' +
-                    err.message);
-                this.pendingResultPromise = null;
-                this.listener.onError(this.defaultResolutionError);
-            });
+                    if (this.latestLookupResult !== null) {
+                        /* We rely here on the assumption that calling this function with
+                         * identical parameters will be essentialy idempotent, and calling
+                         * it with the same address list and a different service config
+                         * should result in a fast and seamless switchover. */
+                        this.listener.onSuccessfulResolution(this.latestLookupResult, this.latestServiceConfig, this.latestServiceConfigError);
+                    }
+                }, err => {
+                    this.latestServiceConfigError = {
+                        code: constants_1.Status.UNAVAILABLE,
+                        details: 'TXT query failed',
+                        metadata: new metadata_1.Metadata(),
+                    };
+                    if (this.latestLookupResult !== null) {
+                        this.listener.onSuccessfulResolution(this.latestLookupResult, this.latestServiceConfig, this.latestServiceConfigError);
+                    }
+                });
+            }
         }
@@ -232,3 +239,3 @@ }
         trace('Resolution update requested for target ' + this.target);
-        if (this.pendingResultPromise === null) {
+        if (this.pendingLookupPromise === null) {
             this.startResolution();
@@ -235,0 +242,0 @@ }

build/src/server.js

@@ -223,2 +223,4 @@ "use strict";
             onSuccessfulResolution: (addressList, serviceConfig, serviceConfigError) => {
+                // We only want one resolution result. Discard all future results
+                resolverListener.onSuccessfulResolution = () => { };
                 if (addressList.length === 0) {
@@ -225,0 +227,0 @@ callback(new Error(`No addresses resolved for port ${port}`), 0);

package.json

 {
   "name": "@grpc/grpc-js",
-  "version": "0.7.3",
+  "version": "0.7.4",
   "description": "gRPC Library for Node - pure JS implementation",
@@ -5,0 +5,0 @@ "homepage": "https://grpc.io/",

src/resolver-dns.ts

@@ -72,30 +72,3 @@ /*
 
-/**
- * Get a promise that always resolves with either the result of the function
- * or the error if it failed.
- * @param fn
- */
-function resolvePromisify<TArg, TResult, TError>(
-  fn: (
-    arg: TArg,
-    callback: (error: TError | null, result: TResult) => void
-  ) => void
-): (arg: TArg) => Promise<TResult | TError> {
-  return arg =>
-    new Promise<TResult | TError>((resolve, reject) => {
-      fn(arg, (error, result) => {
-        if (error) {
-          resolve(error);
-        } else {
-          resolve(result);
-        }
-      });
-    });
-}
-
-const resolveTxtPromise = resolvePromisify<
-  string,
-  string[][],
-  NodeJS.ErrnoException
->(dns.resolveTxt);
+const resolveTxtPromise = util.promisify(dns.resolveTxt);
 const dnsLookupPromise = util.promisify(dns.lookup);
@@ -160,7 +133,7 @@
   private readonly port: string | null;
-  /* The promise results here contain, in order, the A record, the AAAA record,
-   * and either the TXT record or an error if TXT resolution failed */
-  private pendingResultPromise: Promise<
-    [dns.LookupAddress[], string[][] | NodeJS.ErrnoException]
-  > | null = null;
+  private pendingLookupPromise: Promise<dns.LookupAddress[]> | null = null;
+  private pendingTxtPromise: Promise<string[][]> | null = null;
+  private latestLookupResult: TcpSubchannelAddress[] | null = null;
+  private latestServiceConfig: ServiceConfig | null = null;
+  private latestServiceConfigError: StatusObject | null = null;
   private percentage: number;
@@ -194,3 +167,3 @@ private defaultResolutionError: StatusObject;
    * If the target is an IP address, just provide that address as a result.
-   * Otherwise, initiate A, AAAA, and TXT
+   * Otherwise, initiate A, AAAA, and TXT lookups
    */
@@ -206,2 +179,9 @@ private startResolution() {
     if (this.dnsHostname !== null) {
+      /* We clear out latestLookupResult here to ensure that it contains the
+       * latest result since the last time we started resolving. That way, the
+       * TXT resolution handler can use it, but only if it finishes second. We
+       * don't clear out any previous service config results because it's
+       * better to use a service config that's slightly out of date than to
+       * revert to an effectively blank one. */
+      this.latestLookupResult = null;
       const hostname: string = this.dnsHostname;
@@ -212,72 +192,94 @@ /* We lookup both address families here and then split them up later
        * error is indistinguishable from other kinds of errors */
-      const addressResult = dnsLookupPromise(hostname, { all: true });
-      /* We handle the TXT query promise differently than the others because
-       * the name resolution attempt as a whole is a success even if the TXT
-       * lookup fails */
-      const txtResult = resolveTxtPromise(hostname);
-      this.pendingResultPromise = Promise.all([addressResult, txtResult]);
-      this.pendingResultPromise.then(
-        ([addressList, txtRecord]) => {
-          this.pendingResultPromise = null;
-          const ip4Addresses: dns.LookupAddress[] = addressList.filter(
-            addr => addr.family === 4
-          );
-          const ip6Addresses: dns.LookupAddress[] = addressList.filter(addr => addr.family === 6);
-          const allAddresses: TcpSubchannelAddress[] = mergeArrays(
-            ip4Addresses,
-            ip6Addresses
-          ).map(addr => ({ host: addr.address, port: +this.port! }));
-          const allAddressesString: string =
-            '[' +
-            allAddresses.map(addr => addr.host + ':' + addr.port).join(',') +
-            ']';
-          trace(
-            'Resolved addresses for target ' +
-              this.target +
-              ': ' +
-              allAddressesString
-          );
-          if (allAddresses.length === 0) {
-            this.listener.onError(this.defaultResolutionError);
-            return;
-          }
-          let serviceConfig: ServiceConfig | null = null;
-          let serviceConfigError: StatusObject | null = null;
-          if (txtRecord instanceof Error) {
-            serviceConfigError = {
+      this.pendingLookupPromise = dnsLookupPromise(hostname, { all: true });
+      this.pendingLookupPromise.then(addressList => {
+        this.pendingLookupPromise = null;
+        const ip4Addresses: dns.LookupAddress[] = addressList.filter(
+          addr => addr.family === 4
+        );
+        const ip6Addresses: dns.LookupAddress[] = addressList.filter(addr => addr.family === 6);
+        this.latestLookupResult = mergeArrays(
+          ip6Addresses,
+          ip4Addresses
+        ).map(addr => ({ host: addr.address, port: +this.port! }));
+        const allAddressesString: string =
+          '[' +
+          this.latestLookupResult.map(addr => addr.host + ':' + addr.port).join(',') +
+          ']';
+        trace(
+          'Resolved addresses for target ' +
+            this.target +
+            ': ' +
+            allAddressesString
+        );
+        if (this.latestLookupResult.length === 0) {
+          this.listener.onError(this.defaultResolutionError);
+          return;
+        }
+        /* If the TXT lookup has not yet finished, both of the last two
+         * arguments will be null, which is the equivalent of getting an
+         * empty TXT response. When the TXT lookup does finish, its handler
+         * can update the service config by using the same address list */
+        this.listener.onSuccessfulResolution(
+          this.latestLookupResult,
+          this.latestServiceConfig,
+          this.latestServiceConfigError
+        );
+      },
+      err => {
+        trace(
+          'Resolution error for target ' +
+            this.target +
+            ': ' +
+            (err as Error).message
+        );
+        this.pendingLookupPromise = null;
+        this.listener.onError(this.defaultResolutionError);
+      });
+      /* If there already is a still-pending TXT resolution, we can just use
+       * that result when it comes in */
+      if (this.pendingTxtPromise === null) {
+        /* We handle the TXT query promise differently than the others because
+         * the name resolution attempt as a whole is a success even if the TXT
+         * lookup fails */
+        this.pendingTxtPromise = resolveTxtPromise(hostname);
+        this.pendingTxtPromise.then(txtRecord => {
+          this.pendingTxtPromise = null;
+          try {
+            this.latestServiceConfig = extractAndSelectServiceConfig(
+              txtRecord,
+              this.percentage
+            );
+          } catch (err) {
+            this.latestServiceConfigError = {
               code: Status.UNAVAILABLE,
-              details: 'TXT query failed',
+              details: 'Parsing service config failed',
               metadata: new Metadata(),
             };
-          } else {
-            try {
-              serviceConfig = extractAndSelectServiceConfig(
-                txtRecord,
-                this.percentage
-              );
-            } catch (err) {
-              serviceConfigError = {
-                code: Status.UNAVAILABLE,
-                details: 'Parsing service config failed',
-                metadata: new Metadata(),
-              };
-            }
           }
-          this.listener.onSuccessfulResolution(
-            allAddresses,
-            serviceConfig,
-            serviceConfigError
-          );
-        },
-        err => {
-          trace(
-            'Resolution error for target ' +
-              this.target +
-              ': ' +
-              (err as Error).message
-          );
-          this.pendingResultPromise = null;
-          this.listener.onError(this.defaultResolutionError);
-        }
-      );
+          if (this.latestLookupResult !== null) {
+            /* We rely here on the assumption that calling this function with
+             * identical parameters will be essentialy idempotent, and calling
+             * it with the same address list and a different service config
+             * should result in a fast and seamless switchover. */
+            this.listener.onSuccessfulResolution(
+              this.latestLookupResult,
+              this.latestServiceConfig,
+              this.latestServiceConfigError
+            )
+          }
+        }, err => {
+          this.latestServiceConfigError = {
+            code: Status.UNAVAILABLE,
+            details: 'TXT query failed',
+            metadata: new Metadata(),
+          };
+          if (this.latestLookupResult !== null) {
+            this.listener.onSuccessfulResolution(
+              this.latestLookupResult,
+              this.latestServiceConfig,
+              this.latestServiceConfigError
+            )
+          }
+        });
+      }
     }
@@ -288,3 +290,3 @@ }
     trace('Resolution update requested for target ' + this.target);
-    if (this.pendingResultPromise === null) {
+    if (this.pendingLookupPromise === null) {
       this.startResolution();
@@ -291,0 +293,0 @@ }

src/server.ts

@@ -324,2 +324,4 @@ /*
       onSuccessfulResolution: (addressList, serviceConfig, serviceConfigError) => {
+        // We only want one resolution result. Discard all future results
+        resolverListener.onSuccessfulResolution = () => {}
         if (addressList.length === 0) {
@@ -326,0 +328,0 @@ callback(new Error(`No addresses resolved for port ${port}`), 0);

No alert changes

Improved metrics

Total package byte prevSize

853529

0.36%

Lines of code

17918

0.08%

No dependency changes