feat(grpc): gracefully handle subscription network error #509
Conversation
WalkthroughThe changes add graceful network error handling to the gRPC subscription layer with automatic reconnection logic. A React example is updated to demonstrate full SDK initialization and subscription management. The Changes
Sequence DiagramsequenceDiagram
participant App as React App
participant Sub as ToriiSubscription
participant Stream as gRPC Stream
participant Handler as StreamHandler
App->>Sub: initialize subscription<br/>(setupStream(false))
Sub->>Stream: create stream
Stream->>Handler: onMessage
Handler->>Sub: cache lastMessage
alt Network Error
Stream->>Handler: onError (network detected)
Handler->>Sub: trigger reconnect<br/>(setupStream(true))
Sub->>Stream: create new stream
Stream->>Handler: onReconnect callback
Handler->>Sub: re-deliver lastMessage
else Completion
Stream->>Handler: onComplete
Handler->>Sub: cleanup & delete
end
App->>Sub: unmount/cancel
Sub->>Sub: set cancellation flag
Sub->>Stream: cleanup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/grpc/src/torii-client.ts (1)
335-418: Subscriptioncancel()does not stop the underlying stream, and the suggested fix will not workThe core issue is real:
cancel()only flipsisCancelledwithout terminating theServerStreamingCall, allowing the server stream to consume resources. However, the@protobuf-ts/runtime-rpclibrary removed RPC.cancel(); you must use AbortSignal instead via RpcOptions.The suggested
subscription.stream.cancel()call will fail because that method does not exist onServerStreamingCall.The correct approach requires:
- Create an
AbortControllerwhen setting up each stream- Pass its signal via
RpcOptionsto allcreateStream()calls (e.g., insubscribeEntities,subscribeTokens, etc.)- Store the controller so
cancel()can callabortController.abort()- Add the
isCancelledguard inonMessageas originally suggestedThis is more involved than the diff indicates—it requires modifying how each subscription method wires cancellation through to the underlying client calls.
🧹 Nitpick comments (1)
packages/grpc/src/torii-client.ts (1)
171-176: Per‑subscriptionlastMessagetracking is reasonableStoring the last response on
ToriiSubscriptionfor replay on reconnect is a sensible approach; the genericobjecttype is acceptable here, though you could tighten it later by threadingTResthroughToriiSubscriptionif you decide to make it strongly typed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.changeset/tall-birds-thank.md(1 hunks)example/frameworks/react/src/main.tsx(1 hunks)example/package.json(1 hunks)packages/grpc/src/torii-client.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
example/frameworks/react/src/main.tsx (3)
packages/core/src/config/index.ts (1)
createDojoConfig(25-39)packages/grpc/src/torii-client.ts (1)
Subscription(184-206)packages/core/src/provider/DojoProvider.ts (2)
DojoProvider(485-485)DojoProvider(493-494)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-examples
- GitHub Check: build-packages
- GitHub Check: build
🔇 Additional comments (6)
packages/grpc/src/torii-client.ts (2)
138-138: Runtime import ofServerStreamingCalllooks correctSwitching
ServerStreamingCallto a value import is appropriate since you now store and use the streaming call instance at runtime.
208-214:onReconnecthook integrates cleanlyAdding
onReconnecttoStreamHandlerOptionsand invoking it insetupStream(true)provides a clear extension point for callers without affecting existing behavior.example/package.json (1)
25-40: Adding@dojoengine/grpcdependency is consistent with usageIncluding
@dojoengine/grpcas a workspace dependency matches the new imports in the React example and keeps the example package aligned with the SDK/GRPC integration..changeset/tall-birds-thank.md (1)
1-5: Changeset accurately documents the grpc behavior changeThe patch entry and summary line clearly describe the new graceful handling of subscription network errors for
@dojoengine/grpc.example/frameworks/react/src/main.tsx (2)
1-15: Imports and Dojo config wiring look consistentThe new SDK/grpc/starknet imports and the
createDojoConfigusage withmanifest_sepolia.jsonand explicittoriiUrlare coherent with the intended Sepolia/torii setup.
42-68: SDK initialization and rendering flow are soundWrapping SDK initialization in
main(), passingdojoConfig’s torii/worldAddress, and rendering<Root sdk={sdk} />insideStrictModeis a clear and idiomatic bootstrap for the example.
| function Root({ sdk }) { | ||
| const [sub, setSub] = useState<Subscription | null>(null); | ||
|
|
||
| const subscription = useCallback(async () => { | ||
| return await sdk.subscribeEntityQuery({ | ||
| query: new ToriiQueryBuilder().withClause( | ||
| KeysClause([], [], "VariableLen").build() | ||
| ), | ||
| callback: ({ data, error }) => { | ||
| console.log(data, error); | ||
| }, | ||
| }); | ||
| }, [sdk]); | ||
| useEffect(() => { | ||
| subscription().then(([data, sub]) => setSub(sub)); | ||
| return () => { | ||
| if (sub) { | ||
| sub.cancel(); | ||
| } | ||
| }; | ||
| }, [sdk]); |
There was a problem hiding this comment.
🧩 Analysis chain
Effect cleanup never sees the latest subscription; leads to leaked subscriptions
In Root, sub is stored in state but not included in the useEffect dependency array, so the cleanup function always captures the initial sub value (null) and never cancels the actual subscription. This can leave the grpc subscription running after unmount or re‑render.
A simple fix is to keep the subscription in a ref and drive the effect off the memoized subscribe function instead of state:
-import { StrictMode, useCallback, useEffect, useState } from "react";
+import { StrictMode, useCallback, useEffect, useRef } from "react";
@@
-function Root({ sdk }) {
- const [sub, setSub] = useState<Subscription | null>(null);
-
- const subscription = useCallback(async () => {
- return await sdk.subscribeEntityQuery({
+function Root({ sdk }) {
+ const subRef = useRef<Subscription | null>(null);
+
+ const subscribe = useCallback(async () => {
+ const [, sub] = await sdk.subscribeEntityQuery({
query: new ToriiQueryBuilder().withClause(
KeysClause([], [], "VariableLen").build()
),
callback: ({ data, error }) => {
console.log(data, error);
},
});
- }, [sdk]);
- useEffect(() => {
- subscription().then(([data, sub]) => setSub(sub));
- return () => {
- if (sub) {
- sub.cancel();
- }
- };
- }, [sdk]);
+ subRef.current = sub;
+ }, [sdk]);
+
+ useEffect(() => {
+ subscribe().catch(console.error);
+ return () => {
+ subRef.current?.cancel();
+ };
+ }, [subscribe]);This ensures the most recent subscription is always cancelled on cleanup and avoids re‑subscribing due to state changes.
I'll verify the review comment by examining the actual code in the repository.
---
🏁 Script executed:
```shell
#!/bin/bash
# Locate and examine the file in question
fd -t f "main.tsx" --path "*/react/src/*"
Length of output: 233
🏁 Script executed:
#!/bin/bash
# Extract the relevant section from the file
if [ -f "example/frameworks/react/src/main.tsx" ]; then
sed -n '16,36p' "example/frameworks/react/src/main.tsx"
fiLength of output: 641
🏁 Script executed:
#!/bin/bash
# Get broader context around the function to understand imports and full structure
if [ -f "example/frameworks/react/src/main.tsx" ]; then
cat "example/frameworks/react/src/main.tsx"
fiLength of output: 1960
Fix memory leak in useEffect cleanup—subscription never gets cancelled
The bug is confirmed. The cleanup function captures the initial sub value (null) due to missing sub in the dependency array, so the actual subscription object returned by subscription().then() is never cancelled. This leaves the grpc connection open after component unmount or re-render.
The suggested refactor using useRef is the correct approach. Apply the provided diff or equivalent to ensure the most recent subscription is stored in a ref and properly cleaned up on unmount.
🤖 Prompt for AI Agents
In example/frameworks/react/src/main.tsx around lines 16 to 36, the useEffect
cleanup captures a stale `sub` (initial null) because `sub` is not tracked in
deps, so the real subscription never gets cancelled; fix by replacing the local
state subscription tracking with a useRef (e.g., const subRef =
useRef<Subscription | null>(null)), when the subscription promise resolves
assign subRef.current = sub (and cancel any previous subRef.current before
replacing), update the cleanup to cancel subRef.current, and adjust the effect
deps to include only the stable subscription callback (or sdk) so the latest ref
is always used for cleanup.
Closes #
Introduced changes
Checklist
Summary by CodeRabbit
New Features
Dependencies
Documentation