Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incompatibility with [email protected] #555

Closed
aikoven opened this issue Mar 11, 2024 · 4 comments · Fixed by weaviate/typescript-client#157
Closed

Incompatibility with [email protected] #555

aikoven opened this issue Mar 11, 2024 · 4 comments · Fixed by weaviate/typescript-client#157

Comments

@aikoven
Copy link
Contributor

aikoven commented Mar 11, 2024

grpc-js introduced some breaking changes in 1.10.x.

Summary of failing tests
PASS  src/__tests__/clientMiddleware/chain.ts (6.479 s)
PASS  src/__tests__/clientMiddleware/clientStreaming.ts (6.547 s)
FAIL  src/__tests__/unary.ts (6.574 s)
  ● basic

    expect(received).toBe(expected) // Object.is equality

    Expected: false
    Received: true

      40 |           }
      41 |         `);
    > 42 |   expect(serverSignal!.aborted).toBe(false);
        |                                 ^
      43 |
      44 |   channel.close();
      45 |

      at Object.<anonymous> (src/__tests__/unary.ts:42:33)

  ● error

    expect(received).toBe(expected) // Object.is equality

    Expected: false
    Received: true

      228 |     `[ClientError: /nice_grpc.test.Test/TestUnary NOT_FOUND: test]`,
      229 |   );
    > 230 |   expect(serverSignal!.aborted).toBe(false);
          |                                 ^
      231 |
      232 |   expect(trailer?.getAll('test')).toMatchInlineSnapshot(`
      233 |     [

      at Object.<anonymous> (src/__tests__/unary.ts:230:33)

FAIL  src/__tests__/clientStreaming.ts (6.733 s)
  ● basic

    expect(received).toBe(expected) // Object.is equality

    Expected: false
    Received: true

      54 |     }
      55 |   `);
    > 56 |   expect(serverSignal!.aborted).toBe(false);
        |                                 ^
      57 |
      58 |   channel.close();
      59 |

      at Object.<anonymous> (src/__tests__/clientStreaming.ts:56:33)

  ● error

    expect(received).toBe(expected) // Object.is equality

    Expected: false
    Received: true

      284 |   await requestIterableFinish.promise;
      285 |
    > 286 |   expect(serverSignal!.aborted).toBe(false);
          |                                 ^
      287 |
      288 |   expect(trailer?.getAll('test')).toMatchInlineSnapshot(`
      289 |     [

      at Object.<anonymous> (src/__tests__/clientStreaming.ts:286:33)

  ● early response

    expect(received).toBe(expected) // Object.is equality

    Expected: false
    Received: true

      421 |   await requestIterableFinish.promise;
      422 |
    > 423 |   expect(serverSignal!.aborted).toBe(false);
          |                                 ^
      424 |
      425 |   channel.close();
      426 |

      at Object.<anonymous> (src/__tests__/clientStreaming.ts:423:33)

PASS  src/__tests__/serverMiddleware/clientStreaming.ts
PASS  src/__tests__/serverMiddleware/perService.ts
PASS  src/__tests__/ts-proto.ts
PASS  src/__tests__/clientMiddleware/unary.ts
PASS  src/__tests__/serverMiddleware/unary.ts
PASS  src/__tests__/serverMiddleware/chain.ts
PASS  src/__tests__/serverClass.ts
PASS  src/__tests__/defaultCallOptions.ts
PASS  src/__tests__/channel.ts (7.431 s)
FAIL  src/__tests__/clientMiddleware/serverStreaming.ts (21.53 s)
  ● error

    thrown: "Exceeded timeout of 15000 ms for a test.
    Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."

      100 | });
      101 |
    > 102 | test('error', async () => {
          | ^
      103 |   const actions: any[] = [];
      104 |
      105 |   const server = createServer();

      at Object.<anonymous> (src/__tests__/clientMiddleware/serverStreaming.ts:102:1)

FAIL  src/__tests__/clientMiddleware/bidiStreaming.ts (21.546 s)
  ● error

    thrown: "Exceeded timeout of 15000 ms for a test.
    Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."

      110 | });
      111 |
    > 112 | test('error', async () => {
          | ^
      113 |   const actions: any[] = [];
      114 |
      115 |   const server = createServer();

      at Object.<anonymous> (src/__tests__/clientMiddleware/bidiStreaming.ts:112:1)

FAIL  src/__tests__/serverMiddleware/serverStreaming.ts (21.514 s)
  ● error

    thrown: "Exceeded timeout of 15000 ms for a test.
    Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."

      89 | });
      90 |
    > 91 | test('error', async () => {
        | ^
      92 |   const actions: any[] = [];
      93 |
      94 |   const server = createServer().use(

      at Object.<anonymous> (src/__tests__/serverMiddleware/serverStreaming.ts:91:1)

FAIL  src/__tests__/serverMiddleware/bidiStreaming.ts (21.542 s)
  ● error

    thrown: "Exceeded timeout of 15000 ms for a test.
    Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."

      100 | });
      101 |
    > 102 | test('error', async () => {
          | ^
      103 |   const actions: any[] = [];
      104 |
      105 |   const server = createServer().use(

      at Object.<anonymous> (src/__tests__/serverMiddleware/bidiStreaming.ts:102:1)

FAIL  src/__tests__/bidiStreaming.ts (21.711 s)
  ● basic

    expect(received).toBe(expected) // Object.is equality

    Expected: false
    Received: true

      57 |     ]
      58 |   `);
    > 59 |   expect(serverSignal!.aborted).toBe(false);
        |                                 ^
      60 |
      61 |   channel.close();
      62 |

      at Object.<anonymous> (src/__tests__/bidiStreaming.ts:59:33)

  ● error

    thrown: "Exceeded timeout of 15000 ms for a test.
    Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."

      235 | });
      236 |
    > 237 | test('error', async () => {
          | ^
      238 |   const server = createServer();
      239 |
      240 |   let serverSignal: AbortSignal;

      at Object.<anonymous> (src/__tests__/bidiStreaming.ts:237:1)

  ● early response

    expect(received).toBe(expected) // Object.is equality

    Expected: false
    Received: true

      471 |   await requestIterableFinish.promise;
      472 |
    > 473 |   expect(serverSignal!.aborted).toBe(false);
          |                                 ^
      474 |
      475 |   channel.close();
      476 |

      at Object.<anonymous> (src/__tests__/bidiStreaming.ts:473:33)

FAIL  src/__tests__/serverStreaming.ts (22.258 s)
  ● basic

    expect(received).toBe(expected) // Object.is equality

    Expected: false
    Received: true

      53 |     ]
      54 |   `);
    > 55 |   expect(serverSignal!.aborted).toBe(false);
        |                                 ^
      56 |
      57 |   channel.close();
      58 |

      at Object.<anonymous> (src/__tests__/serverStreaming.ts:55:33)

  ● error

    thrown: "Exceeded timeout of 15000 ms for a test.
    Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."

      219 | });
      220 |
    > 221 | test('error', async () => {
          | ^
      222 |   const server = createServer();
      223 |
      224 |   let serverSignal: AbortSignal;

      at Object.<anonymous> (src/__tests__/serverStreaming.ts:221:1)

We should make minimal reproductions and submit them to the grpc-js issues.

Until that, we should pin grpc-js to 1.9.x.

@potkae
Copy link

potkae commented May 31, 2024

Is there any timeline for compatibility with [email protected]?
I'm seeing issues reported in grpc/grpc-node#2647 and there is an attempt to fix those in 1.10.4.

@davidfiala
Copy link
Contributor

To be clear, the issue link mentioned in the comment above appears orthogonal to the compilation error messages which started this specific ticket. Unrelated, that issue link seems unresolved anyhow so it's not clear a 1.10.4 upgrade would help. Though there are other fixes and improvements from the 1.10.x series that need to make it out.

davidfiala added a commit to davidfiala/nice-grpc that referenced this issue Jun 4, 2024
…ures seem to match the output in deeplay-io#555 - work debug from here
@davidfiala
Copy link
Contributor

It appears that in grpc-js 1.10.0 server-interceptors.ts was added which always marks a call as cancelled once it is complete, regardless of success:

https://github.com/grpc/grpc-node/blob/f52d1429fb66ef34c0321d6422c3417972323144/packages/grpc-js/src/server-interceptors.ts#L789 in commit grpc/grpc-node@f52d142

The present 1.10.8 location: https://github.com/grpc/grpc-node/blob/45e5fe5462fea6cb4e3898fa2f07a4836f95916a/packages/grpc-js/src/server-interceptors.ts#L855

Roughly verified this is the cause:

GRPC_TRACE=transport,channel,server_call GRPC_VERBOSITY=DEBUG yarn test src/__tests__/unary.ts -t basic

@davidfiala
Copy link
Contributor

I've asked the grpc-js folks whether the behavior is intentional in grpc/grpc-node#2771

PR #609 ought to resolve both issues I identified in supporting the 1.10.x series.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants