per review comments: (1) grpc-js to ^1.10.8 and remove note about pot…

…entially incompatible 1.10.0 and 1.10.1 (2) document new tests (3) use existing library defer-promise in tests
deeplay-io · Jun 6, 2024 · 685e450 · 685e450
1 parent c189d18
commit 685e450
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 49 deletions.
diff --git a/packages/nice-grpc/package.json b/packages/nice-grpc/package.json
@@ -43,7 +43,7 @@
     "ts-proto": "^1.112.0"
   },
   "dependencies": {
-    "@grpc/grpc-js": "^1.9.5",
+    "@grpc/grpc-js": "^1.10.8",
     "abort-controller-x": "^0.4.0",
     "nice-grpc-common": "^2.0.2"
   }

diff --git a/packages/nice-grpc/src/__tests__/serverStreaming.ts b/packages/nice-grpc/src/__tests__/serverStreaming.ts
@@ -12,35 +12,6 @@ import {TestService} from '../../fixtures/grpc-js/test_grpc_pb';
 import {TestRequest, TestResponse} from '../../fixtures/grpc-js/test_pb';
 import {throwUnimplemented} from './utils/throwUnimplemented';
 
-class AssignablePromise<T> extends Promise<T> {
-  public resolve!: (value: T | PromiseLike<T>) => void;
-  public reject!: (reason?: any) => void;
-
-  private constructor(
-    executor: (
-      resolve: (value: T | PromiseLike<T>) => void,
-      reject: (reason?: any) => void,
-    ) => void,
-  ) {
-    super(executor);
-  }
-
-  static create<T>(): AssignablePromise<T> {
-    let resolveFunc: (value: T | PromiseLike<T>) => void;
-    let rejectFunc: (reason?: any) => void;
-
-    const promise = new AssignablePromise<T>((resolve, reject) => {
-      resolveFunc = resolve;
-      rejectFunc = reject;
-    });
-
-    promise.resolve = resolveFunc!;
-    promise.reject = rejectFunc!;
-
-    return promise;
-  }
-}
-
 function waitForAbort(signal: AbortSignal, timeout: number | undefined = 1000) {
   return new Promise<void>((resolve, reject) => {
     const savedError = new Error('waitForAbort timeout'); // capture stack trace of where the timeout was created
@@ -59,11 +30,15 @@ function waitForAbort(signal: AbortSignal, timeout: number | undefined = 1000) {
   });
 }
 
+/**
+ * Tests that two streaming RPCs one after another work correctly. Specifically tests
+ * that the server does not reuse the same AbortSignal for both RPCs.
+ */
 test('back-to-back', async () => {
   const server = createServer();
 
-  const serverSignal1 = AssignablePromise.create<AbortSignal>();
-  const serverSignal2 = AssignablePromise.create<AbortSignal>();
+  const serverSignal1 = defer<AbortSignal>();
+  const serverSignal2 = defer<AbortSignal>();
 
   let firstTimeOnly = true;
 
@@ -104,7 +79,7 @@ test('back-to-back', async () => {
       },
     }
   `);
-  const serverSig1 = await serverSignal1;
+  const serverSig1 = await serverSignal1.promise;
   expect(serverSig1.aborted).toBe(false);
 
   await expect(it1.next()).resolves.toMatchInlineSnapshot(`
@@ -131,7 +106,7 @@ test('back-to-back', async () => {
       },
     }
   `);
-  const serverSig2 = await serverSignal2;
+  const serverSig2 = await serverSignal2.promise;
   expect(serverSig2.aborted).toBe(false);
 
   await expect(it2.next()).resolves.toMatchInlineSnapshot(`
@@ -151,11 +126,16 @@ test('back-to-back', async () => {
   await server.shutdown();
 });
 
+/**
+ * Tests that two interleaved streaming RPCs work correctly. Specifically tests
+ * that the server does not reuse the same AbortSignal for both RPCs and that
+ * messages from both RPCs are routed correctly.
+ */
 test('interleaved', async () => {
   const server = createServer();
 
-  const serverSignal1 = AssignablePromise.create<AbortSignal>();
-  const serverSignal2 = AssignablePromise.create<AbortSignal>();
+  const serverSignal1 = defer<AbortSignal>();
+  const serverSignal2 = defer<AbortSignal>();
 
   let firstTimeOnly = true;
 
@@ -196,7 +176,7 @@ test('interleaved', async () => {
       },
     }
   `);
-  const serverSig1 = await serverSignal1;
+  const serverSig1 = await serverSignal1.promise;
   expect(serverSig1.aborted).toBe(false);
 
   await expect(it1.next()).resolves.toMatchInlineSnapshot(`
@@ -228,7 +208,7 @@ test('interleaved', async () => {
     }
   `);
 
-  const serverSig2 = await serverSignal2;
+  const serverSig2 = await serverSignal2.promise;
   expect(serverSig1.aborted).toBe(false);
   expect(serverSig2.aborted).toBe(false);
   await expect(it1.next()).resolves.toMatchInlineSnapshot(`

diff --git a/packages/nice-grpc/src/server/createCallContext.ts b/packages/nice-grpc/src/server/createCallContext.ts
@@ -36,10 +36,6 @@ export function createCallContext(call: ServerSurfaceCall): {
       maybeCancel.cancel = undefined;
     });
     call.on('finish', () => {
-      // https://github.com/grpc/grpc-node/issues/2681#issuecomment-1989715667
-      // Versions of grpc-js 1.10.0 and 1.10.1 will not operate correctly with gRPC-JS
-      // and may cause server-side streaming calls to always appear as if they were cancelled
-      // even if the client did not cancel the call.
       maybeCancel.cancel = undefined;
     });
     call.on('cancelled', () => {

diff --git a/yarn.lock b/yarn.lock
@@ -495,7 +495,7 @@
   resolved "https://registry.yarnpkg.com/@esbuild/win32-x64/-/win32-x64-0.18.20.tgz#786c5f41f043b07afb1af37683d7c33668858f6d"
   integrity sha512-kTdfRcSiDfQca/y9QIkng02avJ+NCaQvrMejlsB3RRv5sE9rRoeBPISaZpKxHELzRxZyLvNts1P27W3wV+8geQ==
 
-"@grpc/grpc-js@^1.7.1", "@grpc/grpc-js@^1.9.5":
+"@grpc/grpc-js@^1.10.8", "@grpc/grpc-js@^1.7.1":
   version "1.10.8"
   resolved "https://registry.yarnpkg.com/@grpc/grpc-js/-/grpc-js-1.10.8.tgz#99787785cd8335be861afd1cd485ae9f058e4484"
   integrity sha512-vYVqYzHicDqyKB+NQhAc54I1QWCBLCrYG6unqOIcBTHx+7x8C9lcoLj3KVJXs2VB4lUbpWY+Kk9NipcbXYWmvg==
@@ -3058,9 +3058,9 @@ camelcase@^6.0.0, camelcase@^6.2.0, camelcase@^6.3.0:
   integrity sha512-Gmy6FhYlCY7uOElZUSbxo2UCDH8owEk996gkbrpsgGtrJLM3J7jGxl9Ic7Qwwj4ivOE5AWZWRMecDdF7hqGjFA==
 
 caniuse-lite@^1.0.30001587:
-  version "1.0.30001628"
-  resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001628.tgz#90b6cd85ddc2e9f831de0225f4ca5a130c8d8222"
-  integrity sha512-S3BnR4Kh26TBxbi5t5kpbcUlLJb9lhtDXISDPwOfI+JoC+ik0QksvkZtUVyikw3hjnkgkMPSJ8oIM9yMm9vflA==
+  version "1.0.30001629"
+  resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001629.tgz#907a36f4669031bd8a1a8dbc2fa08b29e0db297e"
+  integrity sha512-c3dl911slnQhmxUIT4HhYzT7wnBK/XYpGnYLOj4nJBaRiw52Ibe7YxlDaAeRECvA786zCuExhxIUJ2K7nHMrBw==
 
 case-anything@^2.1.13:
   version "2.1.13"
@@ -3934,9 +3934,9 @@ ejs@^3.1.7, ejs@^3.1.9:
     jake "^10.8.5"
 
 electron-to-chromium@^1.4.668:
-  version "1.4.790"
-  resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.4.790.tgz#2a3a5509593745c5d65d8acd66b308f2a25da041"
-  integrity sha512-eVGeQxpaBYbomDBa/Mehrs28MdvCXfJmEFzaMFsv8jH/MJDLIylJN81eTJ5kvx7B7p18OiPK0BkC06lydEy63A==
+  version "1.4.792"
+  resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.4.792.tgz#738712f99d02f70c5754ca4264782915fa946849"
+  integrity sha512-rkg5/N3L+Y844JyfgPUyuKK0Hk0efo3JNxUDKvz3HgP6EmN4rNGhr2D8boLsfTV/hGo7ZGAL8djw+jlg99zQyA==
 
 emittery@^0.13.1:
   version "0.13.1"