-
Notifications
You must be signed in to change notification settings - Fork 36
feat(event_processor): add metadata and relay lists #320
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
WalkthroughAdds duplicated subscription-driven integration tests that publish Metadata and Nip65 RelayList updates and assert propagation via state reads. Updates event processor to explicitly dispatch Metadata and RelayList-like kinds to dedicated handlers within the processing loop. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant T as TestClient2
participant R as Relays
participant EP as EventProcessor
participant W as Whitenoise State
rect rgb(240,248,255)
note over T: Publish Metadata
T->>R: Event(Kind::Metadata)
R->>EP: Deliver event
EP->>EP: Match kind == Metadata
EP->>W: handle_metadata(event)
W-->>EP: Ok / Err
EP->>EP: retry if Err per retry_info
end
rect rgb(245,255,250)
note over T: Publish RelayList (Nip65 'R')
T->>R: Event(Kind::RelayList)
R->>EP: Deliver event
EP->>EP: Match kind ∈ {RelayList, InboxRelays, MlsKeyPackageRelays}
EP->>W: handle_relay_list(event)
W-->>EP: Ok / Err
EP->>EP: retry if Err per retry_info
end
note over T,W: Tests assert updated metadata and relay URLs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/whitenoise/event_processor/mod.rs (1)
102-107: Avoid retrying non-transient parse/validation errors from handlersRight now, handler parse failures (e.g., invalid metadata JSON) bubble up as errors and will be retried, which is usually futile. Consider classifying handler errors into retryable vs non-retryable (e.g., map parse/validation issues to WhitenoiseError::InvalidEvent) and short-circuit retries for those.
Example approach (outside this hunk):
- Make handle_metadata return WhitenoiseError::InvalidEvent for parse errors.
- In process_events’ error branch, skip retry when error is InvalidEvent.
Sketch:
// Inside handle_metadata on parse failure: Err(WhitenoiseError::InvalidEvent(format!("Invalid metadata for {}: {}", event.pubkey, e))) // Inside process_events error handling: if let Err(e) = result { match &e { WhitenoiseError::InvalidEvent(_) => { tracing::debug!("Non-retryable error for event kind {:?}: {}", event.kind, e); } _ => { /* existing retry logic */ } } }This avoids wasting retry budget on non-transient failures while keeping transient paths intact.
src/bin/integration_test.rs (1)
330-342: Reduce flakiness: poll until condition instead of fixed sleepsThe fixed 500ms wait can intermittently race with async processing. Prefer polling with a timeout to assert the state changes.
Minimal pattern (outside this hunk):
async fn wait_until<F, Fut>(mut check: F, timeout_ms: u64, poll_ms: u64) -> anyhow::Result<()> where F: FnMut() -> Fut, Fut: std::future::Future<Output = bool>, { let deadline = std::time::Instant::now() + std::time::Duration::from_millis(timeout_ms); while std::time::Instant::now() < deadline { if check().await { return Ok(()); } tokio::time::sleep(tokio::time::Duration::from_millis(poll_ms)).await; } anyhow::bail!("condition not met within {}ms", timeout_ms) }Then, instead of sleeping once, poll:
wait_until( || { let wn = whitenoise.clone(); let pk = account3.pubkey; async move { wn.user_metadata(&pk).await.map(|m| m.name.as_deref() == Some("Known User Sub Update")).unwrap_or(false) } }, 4000, 100, ).await.unwrap(); // Similarly for relay membership: wait_until( || { let wn = whitenoise.clone(); let user = user3.clone(); let parsed_new = RelayUrl::parse(&new_relay_url).unwrap(); async move { wn.user_relays(&user, whitenoise::RelayType::Nip65).await .map(|rs| rs.iter().any(|r| r.url == parsed_new)) .unwrap_or(false) } }, 4000, 100, ).await.unwrap();This will make the test far more resilient across different environments and load conditions.
Also applies to: 348-357
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/bin/integration_test.rs(1 hunks)src/whitenoise/event_processor/mod.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/bin/integration_test.rs
📄 CodeRabbit Inference Engine (.cursor/rules/integration-test.mdc)
The integration test in src/bin/integration_test.rs should ALWAYS be run with the
just int-testcommand and not run on its own with different log and data directories.
Files:
src/bin/integration_test.rs
🧬 Code Graph Analysis (2)
src/whitenoise/event_processor/mod.rs (3)
src/whitenoise/event_processor/event_handlers/handle_metadata.rs (1)
handle_metadata(7-26)src/nostr_manager/mod.rs (1)
event(884-889)src/whitenoise/event_processor/event_handlers/handle_relay_list.rs (1)
handle_relay_list(9-20)
src/bin/integration_test.rs (3)
src/whitenoise/relays.rs (1)
new(68-75)src/whitenoise/users.rs (1)
new(19-27)src/nostr_manager/mod.rs (1)
new(79-188)
⏰ 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). (1)
- GitHub Check: check (ubuntu-latest, native)
🔇 Additional comments (3)
src/whitenoise/event_processor/mod.rs (1)
102-107: Explicit dispatch for Metadata and RelayList-like kinds: LGTMRouting Kind::Metadata to handle_metadata and RelayList | InboxRelays | MlsKeyPackageRelays to handle_relay_list is correct and aligns with the handlers’ semantics. This integrates cleanly into the existing retry/error flow.
src/bin/integration_test.rs (2)
294-359: Subscription-driven Metadata + NIP-65 relay updates validation: LGTMNicely exercises the new event routing end-to-end. Publishing a fresh Metadata and a NIP-65 RelayList and then asserting state via user_metadata and user_relays is a good, realistic verification of the processor.
294-299: Reminder: run this integration test viajust int-testPer repo guidelines, this binary integration test should be executed with the just int-test command to ensure consistent data/log directories and environment. Running it ad hoc can lead to state leakage and false positives (e.g., the relay URL check).
delcin-raj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Adds metadata and relay lists events processing to event processor
Summary by CodeRabbit