-
Notifications
You must be signed in to change notification settings - Fork 135
fix(rivetkit): uri encode actor ids to fix cloudflare workers actor id with generation #3504
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
Code ReviewSummaryThis PR adds URL encoding for actor IDs in WebSocket protocol headers to fix an issue with Cloudflare Workers when actor IDs contain colons (specifically the generation format: Positive Aspects ✅
Issues & Recommendations 🔍1. Minor: Inconsistent Variable NamingIn // Current
let actor_id_raw = protocols...
let actor_id = urlencoding::decode(actor_id_raw)...
// Suggestion (more explicit)
let actor_id_encoded = protocols...
let actor_id = urlencoding::decode(actor_id_encoded)...2. Debugging Improvements in Test CodeThe change in
3. Missing
|
More templates
@rivetkit/cloudflare-workers
@rivetkit/db
@rivetkit/framework-base
@rivetkit/next-js
@rivetkit/react
rivetkit
@rivetkit/sql-loader
@rivetkit/engine-runner
@rivetkit/engine-runner-protocol
commit: |
4dbfcc3 to
9faab1f
Compare
763302d to
57ae58f
Compare
…d with generation
9faab1f to
1431ddd
Compare
57ae58f to
6557c65
Compare
PR Review: URI Encoding for Actor IDs with GenerationSummaryThis PR fixes an important bug where actor IDs containing colons (from the generation suffix format Code Quality: ✅ GoodStrengths:
Code Style Adherence:
Potential Issues:
|
Code ReviewSummaryThis PR fixes a critical issue where actor IDs containing colons (format: ✅ Positive Aspects
🔍 Code Quality Issues1. Redundant
|
Code ReviewSummaryThis PR fixes an important bug where actor IDs containing colons (format: Positive Aspects ✅
Issues & Concerns 🔍Critical: Missing Rust Client Update
|
Pull Request ReviewSummaryThis PR fixes a bug where actor IDs containing colons (from generation tracking) were breaking Cloudflare Workers WebSocket connections. The fix properly URL-encodes actor IDs when passing them via WebSocket protocol headers. Code Quality ✅Strengths:
Minor observations:
Potential Issues ❌Critical - Missing Error Handling: In let actor_id = urlencoding::decode(actor_id_raw)
.context("invalid url encoding in actor id")?
.to_string();This will return an error if the actor ID is not properly URL-encoded. However, this creates a backwards compatibility issue:
Recommendation: Consider adding backwards compatibility handling: // Try to decode, but fall back to raw string for backwards compatibility
let actor_id = urlencoding::decode(actor_id_raw)
.map(|s| s.to_string())
.unwrap_or_else(|_| {
tracing::warn\!(actor_id_raw, "actor id not url-encoded, using raw value for backwards compatibility");
actor_id_raw.to_string()
});Or, if you want to enforce the new behavior, document this as a breaking change. Performance Considerations ✅
Security Concerns
|
Merge activity
|
…d with generation (#3504)

No description provided.