fix(pkg/da): fallback to polling when ws cannot connect#3211
fix(pkg/da): fallback to polling when ws cannot connect#3211julienrbrt merged 3 commits intomainfrom
Conversation
|
The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).
|
|
Claude finished @julienrbrt's task in 1m 35s —— View job Code Review
The fix is correct and addresses the regression cleanly. A few observations:
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 19 minutes and 42 seconds. ⌛ 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. 📝 WalkthroughWalkthroughMultiple application entry points and the blob DA JSON-RPC client are updated to pass a configured logger parameter to Changes
Sequence DiagramsequenceDiagram
actor Caller
participant NewWSClient
participant WSEndpoint as WebSocket<br/>Endpoint
participant Logger
participant HTTPClient as HTTP Client<br/>(Fallback)
Caller->>NewWSClient: NewWSClient(ctx, logger, addr, token, authHeaderName)
NewWSClient->>WSEndpoint: Attempt WebSocket connection<br/>via httpToWS(addr)
alt WebSocket Connection Succeeds
WSEndpoint-->>NewWSClient: Connected
NewWSClient-->>Caller: Return WebSocket Client
else WebSocket Connection Fails
WSEndpoint-->>NewWSClient: Connection Error
NewWSClient->>Logger: Log Warning
Logger-->>NewWSClient: Logged
NewWSClient->>HTTPClient: Create HTTP Client<br/>via NewClient(addr)
HTTPClient-->>NewWSClient: HTTP Client Created
NewWSClient-->>Caller: Return HTTP Client (Fallback)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3211 +/- ##
==========================================
- Coverage 61.43% 61.41% -0.02%
==========================================
Files 120 120
Lines 12470 12474 +4
==========================================
Hits 7661 7661
- Misses 3949 3953 +4
Partials 860 860
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Overview
Follow up of #3146.
Enforcing DA to be available at connection effectively makes the node require DA to be working prior to starting the node.
It is a regression. We should allow DA to be unavailable when starting. This does mean it will fully rely on polling when it will be back up instead of WS.
No changelog needed because not released.
Summary by CodeRabbit
Bug Fixes
Improvements