fix: Don't close client if we've already aborted (#968)

stephenh · Nov 28, 2023 · 7ee1507 · 7ee1507
1 parent 105e5fd
commit 7ee1507
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 26 deletions.
diff --git a/integration/grpc-web-abort-signal/example.ts b/integration/grpc-web-abort-signal/example.ts
@@ -1081,17 +1081,23 @@ export class GrpcWebImpl {
             }
           },
         });
-        observer.add(() => {
-          if (!abortSignal || !abortSignal.aborted) {
-            return client.close();
-          }
-        });
 
         if (abortSignal) {
-          abortSignal.addEventListener("abort", () => {
+          const abort = () => {
             observer.error(abortSignal.reason);
             client.close();
+          };
+          abortSignal.addEventListener("abort", abort);
+          observer.add(() => {
+            if (abortSignal.aborted) {
+              return;
+            }
+
+            abortSignal.removeEventListener("abort", abort);
+            client.close();
           });
+        } else {
+          observer.add(() => client.close());
         }
       });
       upStream();

diff --git a/integration/grpc-web/example.ts b/integration/grpc-web/example.ts
@@ -1028,9 +1028,7 @@ export class GrpcWebImpl {
             }
           },
         });
-        observer.add(() => {
-          return client.close();
-        });
+        observer.add(() => client.close());
       });
       upStream();
     }).pipe(share());

diff --git a/src/generate-grpc-web.ts b/src/generate-grpc-web.ts
@@ -336,14 +336,6 @@ function createInvokeMethod(ctx: Context) {
   const { options } = ctx;
   const { useAbortSignal } = options;
 
-  const maybeAbortSignal = useAbortSignal
-    ? `
-      if (abortSignal) abortSignal.addEventListener("abort", () => {
-        observer.error(abortSignal.reason);
-        client.close();
-      });`
-    : "";
-
   return code`
     invoke<T extends UnaryMethodDefinitionish>(
       methodDesc: T,
@@ -380,16 +372,29 @@ function createInvokeMethod(ctx: Context) {
               }
             },
           });
-          observer.add(() => {
-           ${
-             !useAbortSignal
-               ? `return client.close();`
-               : `if (!abortSignal || !abortSignal.aborted) 
-              return client.close();`
-           }
-          });
+          ${
+            useAbortSignal
+              ? `
+          if (abortSignal) {
+            const abort = () => {
+              observer.error(abortSignal.reason);
+              client.close();
+            };
+            abortSignal.addEventListener("abort", abort);
+            observer.add(() => {
+              if (abortSignal.aborted) {
+                return;
+              }
 
-          ${maybeAbortSignal}
+              abortSignal.removeEventListener('abort', abort); 
+              client.close();
+            });
+          } else {
+            observer.add(() => client.close());
+          }
+          `
+              : `observer.add(() => client.close());`
+          }
         });
         upStream();
       }).pipe(${share}());