Merge pull request #1512 from murgatroid99/grpc-js_default_service_co…

…nfig grpc-js: Fix handling of unsuccessful TXT record lookups
grpc · Jul 23, 2020 · 0236633 · 0236633
2 parents 3ddfd37 + f05e9fb
commit 0236633
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 27 deletions.
diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@grpc/grpc-js",
-  "version": "1.1.2",
+  "version": "1.1.3",
   "description": "gRPC Library for Node - pure JS implementation",
   "homepage": "https://grpc.io/",
   "repository": "https://github.com/grpc/grpc-node/tree/master/packages/grpc-js",

diff --git a/packages/grpc-js/src/resolver-dns.ts b/packages/grpc-js/src/resolver-dns.ts
@@ -237,19 +237,13 @@ class DnsResolver implements Resolver {
             }
           },
           (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,
-                {}
-              );
-            }
+            /* If TXT lookup fails we should do nothing, which means that we
+             * continue to use the result of the most recent successful lookup,
+             * or the default null config object if there has never been a
+             * successful lookup. We do not set the latestServiceConfigError
+             * here because that is specifically used for response validation
+             * errors. We still need to handle this error so that it does not
+             * bubble up as an unhandled promise rejection. */
           }
         );
       }

diff --git a/packages/grpc-js/src/resolving-load-balancer.ts b/packages/grpc-js/src/resolving-load-balancer.ts
@@ -59,11 +59,10 @@ export class ResolvingLoadBalancer implements LoadBalancer {
   private currentState: ConnectivityState = ConnectivityState.IDLE;
   /**
    * The service config object from the last successful resolution, if
-   * available. A value of undefined indicates that there has not yet
-   * been a successful resolution. A value of null indicates that the last
-   * successful resolution explicitly provided a null service config.
+   * available. A value of null indicates that we have not yet received a valid
+   * service config from the resolver.
    */
-  private previousServiceConfig: ServiceConfig | null | undefined = undefined;
+  private previousServiceConfig: ServiceConfig | null = null;
 
   /**
    * The backoff timer for handling name resolution failures.
@@ -131,19 +130,13 @@ export class ResolvingLoadBalancer implements LoadBalancer {
           // Step 4 and 5
           if (serviceConfigError === null) {
             // Step 5
-            this.previousServiceConfig = serviceConfig;
+            this.previousServiceConfig = null;
             workingServiceConfig = this.defaultServiceConfig;
           } else {
             // Step 4
-            if (this.previousServiceConfig === undefined) {
+            if (this.previousServiceConfig === null) {
               // Step 4.ii
-              if (this.defaultServiceConfig === null) {
-                // Step 4.ii.b
-                this.handleResolutionFailure(serviceConfigError);
-              } else {
-                // Step 4.ii.a
-                workingServiceConfig = this.defaultServiceConfig;
-              }
+              this.handleResolutionFailure(serviceConfigError);
             } else {
               // Step 4.i
               workingServiceConfig = this.previousServiceConfig;