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

Fix: The deadline passed to 'waitForReady' in "GrpcClient", was a millisecond gap, but the method needs a date or a TS of the deadline #148

Merged
merged 1 commit into from
May 6, 2024

Conversation

lsidoree
Copy link
Contributor

@lsidoree lsidoree commented May 6, 2024

"waitForReady" was called with a deadline of 10000 milliseconds.

But the method 'watchConnectivityState' used by 'waitForReady', needs a deadline date.
cf: https://github.com/grpc/grpc-node/blob/%40grpc/grpc-js%401.9.7/packages/grpc-js/src/internal-channel.ts#L739

This caused to throw error "Deadline passed without connectivity state change" in "watchConnectivityState" every time.

So this fix is to set the deadline date at now + 10s

"waitForReady" was called with a deadline of 10000 milliseconds.

But the method 'watchConnectivityState' used by 'waitForReady', wants a deadline date.
cf: https://github.com/grpc/grpc-node/blob/%40grpc/grpc-js%401.9.7/packages/grpc-js/src/internal-channel.ts#L739

So this fix is to set the deadline date at now + 10s
@CLAassistant
Copy link

CLAassistant commented May 6, 2024

CLA assistant check
All committers have signed the CLA.

@lsidoree lsidoree changed the title The deadline passed to waitForReady was miliseconds, but waitForReady waits for a date or a TS of the deadline Fix: The deadline passed to 'waitForReady' in "GrpcClient", was a millisecond gap, but the method needs a date or a TS of the deadline May 6, 2024
@lsidoree lsidoree marked this pull request as draft May 6, 2024 16:34
@jwulf jwulf changed the title Fix: The deadline passed to 'waitForReady' in "GrpcClient", was a millisecond gap, but the method needs a date or a TS of the deadline fix(zeebe): deadline passed to 'waitForReady' in "GrpcClient" needs a date May 6, 2024
@jwulf jwulf marked this pull request as ready for review May 6, 2024 16:41
@jwulf jwulf changed the title fix(zeebe): deadline passed to 'waitForReady' in "GrpcClient" needs a date Fix: The deadline passed to 'waitForReady' in "GrpcClient", was a millisecond gap, but the method needs a date or a TS of the deadline May 6, 2024
@jwulf jwulf merged commit 12db206 into camunda:main May 6, 2024
5 of 10 checks passed
jwulf added a commit that referenced this pull request May 6, 2024
jwulf added a commit that referenced this pull request May 6, 2024
@jwulf
Copy link
Member

jwulf commented May 6, 2024

Thanks for finding and fixing this @lsidoree.

For future reference, pull requests should be made against the alpha branch. See https://github.com/camunda/camunda-8-js-sdk/blob/main/CONTRIBUTING.md.

I can roll this fix into another patch - or if you'd like to submit a PR against alpha you are welcome to.

@lsidoree
Copy link
Contributor Author

lsidoree commented May 7, 2024

Next time I'll read the contribution rules better, sorry!

@lsidoree lsidoree deleted the patch-1 branch May 7, 2024 07:27
github-actions bot pushed a commit that referenced this pull request May 7, 2024
## [8.5.2](v8.5.1...v8.5.2) (2024-05-07)

### Bug Fixes

* **zeebe:** waitForReady deadline not miliseconds, but date ([#148](#148)) ([12db206](12db206))

### Features

* **repo:** add CAMUNDA_CUSTOM_ROOT_CERT_STRING parameter ([7451a66](7451a66)), closes [#151](#151) [#150](#150) [#146](#146) [#142](#142) [#151](#151) [#150](#150) [#142](#142) [#151](#151) [#150](#150) [#142](#142) [#151](#151) [#150](#150) [#142](#142) [#151](#151) [#150](#150) [#142](#142) [#151](#151) [#150](#150) [#142](#142)

### Reverts

* Revert "fix(zeebe): waitForReady deadline not miliseconds, but date (#148)" (#149) ([f8c0c7d](f8c0c7d)), closes [#148](#148) [#149](#149)
github-actions bot pushed a commit that referenced this pull request May 7, 2024
## [8.5.2](v8.5.1...v8.5.2) (2024-05-07)

### Bug Fixes

* **zeebe:** waitForReady deadline not miliseconds, but date ([#148](#148)) ([12db206](12db206))

### Features

* **repo:** add CAMUNDA_CUSTOM_ROOT_CERT_STRING parameter ([7451a66](7451a66)), closes [#151](#151) [#150](#150) [#146](#146) [#142](#142) [#151](#151) [#150](#150) [#142](#142) [#151](#151) [#150](#150) [#142](#142) [#151](#151) [#150](#150) [#142](#142) [#151](#151) [#150](#150) [#142](#142) [#151](#151) [#150](#150) [#142](#142)

### Reverts

* Revert "fix(zeebe): waitForReady deadline not miliseconds, but date (#148)" (#149) ([f8c0c7d](f8c0c7d)), closes [#148](#148) [#149](#149)
@jwulf
Copy link
Member

jwulf commented May 7, 2024

Thank you for finding and fixing this @lsidoree. This has been a bug in the zeebe client for the last five years. How did you find it?

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 this pull request may close these issues.

None yet

3 participants