Skip to content

Feat/zrpc#70

Merged
kooksee merged 14 commits into
v2from
feat/zrpc
Jun 17, 2026
Merged

Feat/zrpc#70
kooksee merged 14 commits into
v2from
feat/zrpc

Conversation

@kooksee

@kooksee kooksee commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces zrpc, a lightweight protobuf RPC framework over NATS request-reply, complete with a code generator (protoc-gen-zrpc-go), client/server runtimes, documentation, and a demo application. The code review highlights several critical and high-severity issues regarding concurrency and context handling. Specifically, a deferred cancel function in OpenStream immediately invalidates the returned stream's context, while several methods in the client implementation lack proper mutex locking, leading to data races and potential nil pointer panics. Additionally, double-locking patterns in stream close methods present race conditions, the health check flush ignores context deadlines, and a missing defensive check in the unary handler could trigger a panic on nil middleware responses.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread pkg/zrpc/client.go Outdated
Comment thread clients/zrpcc/client.go Outdated
Comment thread clients/zrpcc/client.go Outdated
Comment thread clients/zrpcc/client.go
Comment thread pkg/zrpc/client.go
Comment thread pkg/zrpc/server.go
Comment thread clients/zrpcc/client.go Outdated
Comment thread pkg/zrpc/server.go
@kooksee kooksee marked this pull request as ready for review June 15, 2026 14:27
kooksee and others added 10 commits June 15, 2026 22:36
Separate OpenStream handshake timeout from stream lifecycle context, harden zrpcc mutex usage, refactor stream Close helpers, and guard unary middleware nil responses.

Co-authored-by: Cursor <cursoragent@cursor.com>
Publish cross-platform binaries on v* tags with cliff-generated release notes.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Propagate stream ACK/CloseSend failures to clients, add stream deadlines and safer payloads, protect zrpcc with RWMutex, run zrpc tests against NATS on v2 CI.

Co-authored-by: Cursor <cursoragent@cursor.com>
Mark zrpcc as closed before reconnecting, fix ineffectual stream context assignment, and check Unsubscribe error in defer.

Co-authored-by: Cursor <cursoragent@cursor.com>
Expand zrpc docs for streaming, logging, and CI; add package comments and zrpcc/zrpcs connect lifecycle logs.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Document zrpc release scope in Keep a Changelog format and filter noisy commits in git-cliff.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Close an optional Started channel after NATS subscriptions are flushed
so tests do not race the zrpc server startup in CI.

Co-authored-by: Cursor <cursoragent@cursor.com>
@kooksee kooksee merged commit 6c1999b into v2 Jun 17, 2026
2 checks passed
@kooksee kooksee deleted the feat/zrpc branch June 17, 2026 13:16
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.

1 participant