feat: migrate RPC surface to Connect-RPC#111
Conversation
- plugin.go RPC() now returns (string, http.Handler) using the generated tcpV1connect.NewTCPServiceHandler from api-go v6.0.0-beta.9 - rpc.go reshaped to the Connect handler interface; the single Close method takes tcpV1.CloseRequest and returns tcpV1.Response - tests/tcp_plugin_test.go closeConn helper switched from goridge codec to the Connect client over h2c - new tests/tcp_api_test.go adds 3-protocol coverage (Connect, plain HTTP + protojson, plain gRPC) against the same handler - new tests/configs/.rr-tcp-api.yaml minimal config for the new tests - bump api-go to v6.0.0-beta.9, goridge to v4.0.0-beta.2, rpc/v6 to v6.0.0-beta.4
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR migrates the TCP plugin's RPC interface from a goridge-based net/rpc model to Connect-RPC HTTP handlers. The plugin's ChangesConnect-RPC Handler Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Migrates the tcp plugin’s RPC API from the legacy net/rpc + goridge codec to a Connect-RPC HTTP/2 (h2c) handler generated from api-go/v6, and updates/adds tests to exercise the new wire surface (Connect, plain JSON over HTTP/1.1, and gRPC).
Changes:
- Switch tcp plugin RPC registration to return a Connect-RPC
http.Handler(generated handler) and update the RPC implementation to Connect request/response types. - Update existing tcp plugin tests to call
Closevia a Connect client instead ofnet/rpc. - Add a new integration test + minimal RR config to validate Connect, plain HTTP+JSON, and gRPC access on the same
rpc.listenport.
Reviewed changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
rpc.go |
Replaces legacy RPC method signature with Connect-RPC request/response types and error mapping. |
plugin.go |
Changes RPC() to return a Connect-RPC handler (string + http.Handler) for registration with the RPC plugin. |
tests/tcp_plugin_test.go |
Rewrites closeConn helper to use a Connect client over h2c. |
tests/tcp_api_test.go |
Adds protocol-surface tests for Connect, HTTP/1.1 JSON, and gRPC. |
tests/configs/.rr-tcp-api.yaml |
Adds minimal RR config used by the new API-surface test. |
tests/go.mod / tests/go.sum |
Adds/bump test-module dependencies needed for Connect + gRPC clients. |
go.mod / go.sum |
Bumps goridge/v4 dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- drop TestTCPConnectAPI doc (what-narration; name+imports already say it) - soften TestTCPHTTPApi doc to "non-Connect HTTP client" (Guzzle/curl was an unverified-in-repo specifier)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@go.mod`:
- Line 10: The dependency entry for module
github.com/roadrunner-server/goridge/v4 is using a non-existent tag
v4.0.0-beta.2; update that require line to the valid release v4.0.0-beta.1 so
the go.mod references the existing version (ensure you run `go get`/`go mod
tidy` afterwards to update go.sum and vendor if needed).
In `@tests/tcp_api_test.go`:
- Around line 59-65: The test currently calls wg.Go(...) which doesn't exist on
sync.WaitGroup; replace that with wg.Add(1) before spawning a goroutine and use
go func() { defer wg.Done(); ... }() to run the select body; locate the block
around wg, ch and stop in tests/tcp_api_test.go (the anonymous goroutine using
wg.Go) and change it to increment the wait group with wg.Add(1), start a plain
go func(), and ensure defer wg.Done() is the first line inside the goroutine so
the wait group is decremented when the goroutine exits.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e361ec3e-0a1a-47f0-96b2-ed380d7fac8f
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumtests/go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
go.modplugin.gorpc.gotests/configs/.rr-tcp-api.yamltests/go.modtests/tcp_api_test.gotests/tcp_plugin_test.go
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #111 +/- ##
=============================
=============================
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Catches drift in transitive deps + server/v6 beta.5 → beta.6 in tests/.
Summary
Migrates the tcp plugin's RPC surface from net/rpc + goridge codec to Connect-RPC over HTTP/2 (h2c) using
tcpV1connect.NewTCPServiceHandlerfrom api-go v6.0.0-beta.9.The single RPC method
Close(uuid)is now reachable via three wire protocols on the samerpc.listenport:tcpV1connect.NewTCPServiceClient)The existing
closeConnhelper intcp_plugin_test.gowas rewritten to use the Connect client. A newtests/tcp_api_test.gocovers all three wire protocols, withtests/configs/.rr-tcp-api.yamlproviding a minimal container config.Breaking changes
Wire protocol change: callers using
net/rpc+ goridge codec (client.Call("tcp.Close", uuid, &ret)) must switch to a Connect / HTTP / gRPC client targeting the samerpc.listenport. The path/tcp.v1.TCPService/Closeaccepts plain JSON over POST for clients without a Connect SDK.Dep bumps:
github.com/roadrunner-server/api-go/v6→v6.0.0-beta.9github.com/roadrunner-server/goridge/v4→v4.0.0-beta.2(note:pkg/rpcis gone in this release)github.com/roadrunner-server/rpc/v6(tests) →v6.0.0-beta.4Summary by CodeRabbit
New Features
Tests
Chores