fix: realtime late join #6869

Septias · 2025-05-21T08:57:43Z

This PR adds a test to reproduce a bug raised by @adbenitez that peer channels break when the resend feature is used.

WofWca · 2025-05-22T20:34:54Z

This is still WIP? This only adds tests.

Septias · 2025-07-07T11:13:33Z

Found out that somehow the wrong iroh topic is transmitted to fiona and that is why she fails to join. In every send invocation a new topic is generated, and we use resend...

Septias · 2025-07-08T09:47:52Z

@link2xt I made some unrelated changes, which are easy to track in the individual commits, but they pollute the final PR. Would you prefer to split this into a refactor and bug-fix PR?

link2xt · 2025-07-09T20:00:41Z

I made some unrelated changes, which are easy to track in the individual commits, but they pollute the final PR. Would you prefer to split this into a refactor and bug-fix PR?

If it's just these documentation updates then no need to split this.

src/peer_channels.rs

Septias · 2025-07-10T10:24:54Z

It seems like the test is a bit flaky (for me it ran 7/8 times), but I do not know a fix of the top of my head yet.. Maybe we can just check how often it actually happens and fix it if neccessary in a follow up PR

src/peer_channels.rs

iequidoo · 2025-07-15T16:43:13Z

src/mimefactory.rs

@@ -1492,7 +1499,7 @@ impl MimeFactory {
            }
            SystemMessage::IrohNodeAddr => {
                headers.push((
-                    "Iroh-Node-Addr",
+                    HeaderDef::IrohNodeAddr.into(),


.get_headername() would read more straightforwardly (and we use it in many other places). But probably we should just rename HeaderDef to HeaderName, the current naming looks unclear.

iequidoo · 2025-07-15T16:43:16Z

src/peer_channels.rs

@@ -109,7 +109,7 @@ impl Iroh {

        info!(
            ctx,
-            "IROH_REALTIME: Joining gossip with peers: {:?}", node_ids,
+            "IROH_REALTIME: Joining gossip {topic} with peers: {:?}", node_ids,


Suggested change

"IROH_REALTIME: Joining gossip {topic} with peers: {:?}", node_ids,

"IROH_REALTIME: Joining gossip {topic} with peers: {:?}.", node_ids,

iequidoo · 2025-07-15T16:43:19Z

src/peer_channels.rs

@@ -198,7 +192,7 @@ impl Iroh {
    }

    /// Leave the realtime channel for a given topic.
-    pub(crate) async fn leave_realtime(&self, topic: TopicId) -> Result<()> {
+    pub async fn leave_realtime(&self, topic: TopicId) -> Result<()> {


Why it needs to be public now?

iequidoo · 2025-07-15T16:43:23Z

src/peer_channels.rs

+        if let Some(iroh) = &*self.iroh.read().await {
+            info!(
+                self,
+                "Adding (maybe existing) peer with id {} to {topic}", peer.node_id


Suggested change

"Adding (maybe existing) peer with id {} to {topic}", peer.node_id

"Adding (maybe existing) peer with id {} to {topic}.", peer.node_id

iequidoo · 2025-07-15T16:43:26Z

src/peer_channels.rs

 }

 /// Cache a peers [NodeId] for one topic.
-pub(crate) async fn iroh_add_peer_for_topic(
+pub async fn iroh_add_peer_for_topic(


The same question

iequidoo · 2025-07-15T16:43:30Z

src/peer_channels.rs

@@ -336,6 +341,7 @@ pub(crate) async fn iroh_add_peer_for_topic(
 }

 /// Add gossip peer from `Iroh-Node-Addr` header to WebXDC message identified by `instance_id`.
+/// This should not start iroh, because receiving a NodeAddr does not mean you want to participate.


Suggested change

/// This should not start iroh, because receiving a NodeAddr does not mean you want to participate.

/// This does not start Iroh, because receiving a NodeAddr does not mean you want to participate.

Also, why is this public?

iequidoo · 2025-07-15T16:43:34Z

src/peer_channels.rs

    Ok(())
 }

 /// Insert topicId into the database so that we can use it to retrieve the topic.
-pub(crate) async fn insert_topic_stub(ctx: &Context, msg_id: MsgId, topic: TopicId) -> Result<()> {
+pub async fn insert_topic_stub(ctx: &Context, msg_id: MsgId, topic: TopicId) -> Result<()> {


The same question

iequidoo · 2025-07-15T16:43:39Z

src/peer_channels.rs

+        let msg = alice.pop_sent_msg().await;
+        let fiona_instance = fiona.recv_msg(&msg).await;
+        fiona_instance.chat_id.accept(fiona).await.unwrap();
+


There should be a check that peer channels aren't initialized, otherwise the test would pass w/o the fix

iequidoo · 2025-07-15T16:43:42Z

src/peer_channels.rs

+    async fn connect_alice_bob(
+        alice: &mut TestContext,
+        bob: &mut TestContext,
+        chat: ChatId,


It should be clear that it's Alice's ChatId, renaming it or reordering parameters would make sense

Septias marked this pull request as draft May 23, 2025 07:57

link2xt force-pushed the main branch from 285d80a to 416131b Compare June 26, 2025 14:07

Septias force-pushed the sk/test_webxdc_resend branch 2 times, most recently from fa9afd2 to ed12f9a Compare July 8, 2025 08:24

Septias marked this pull request as ready for review July 8, 2025 12:39

link2xt reviewed Jul 9, 2025

View reviewed changes

src/peer_channels.rs Outdated Show resolved Hide resolved

Septias added 12 commits July 10, 2025 09:56

add test

3618321

fix test

bb5c107

Do not use static strings

495481b

capitalize Iroh

63ba7ef

don't start iroh when adding peers late

44cb66c

change function visibility

e199f20

improve logs

9e5a4d4

fix: use initial topic on resend

082ac3a

fixup rebase

bad8676

clippy fixes

0e01fa1

enable last check

3d9486e

remove timezone (rebase artifact)

2d12aec

Septias force-pushed the sk/test_webxdc_resend branch from 1c471ed to 2d12aec Compare July 10, 2025 07:58

update comment

d08b828

link2xt reviewed Jul 13, 2025

View reviewed changes

src/peer_channels.rs Outdated Show resolved Hide resolved

remove timout

a8da677

Septias requested review from link2xt and iequidoo July 14, 2025 10:03

iequidoo reviewed Jul 15, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix: realtime late join #6869

fix: realtime late join #6869

Septias commented May 21, 2025

Uh oh!

WofWca commented May 22, 2025

Uh oh!

Septias commented Jul 7, 2025 •

edited

Loading

Uh oh!

Septias commented Jul 8, 2025

Uh oh!

link2xt commented Jul 9, 2025

Uh oh!

Uh oh!

Septias commented Jul 10, 2025

Uh oh!

Uh oh!

iequidoo Jul 15, 2025

Uh oh!

iequidoo Jul 15, 2025

Uh oh!

iequidoo Jul 15, 2025

Uh oh!

iequidoo Jul 15, 2025

Uh oh!

iequidoo Jul 15, 2025

Uh oh!

iequidoo Jul 15, 2025

Uh oh!

iequidoo Jul 15, 2025

Uh oh!

iequidoo Jul 15, 2025

Uh oh!

iequidoo Jul 15, 2025

Uh oh!

Uh oh!

	"IROH_REALTIME: Joining gossip {topic} with peers: {:?}", node_ids,
	"IROH_REALTIME: Joining gossip {topic} with peers: {:?}.", node_ids,

	"Adding (maybe existing) peer with id {} to {topic}", peer.node_id
	"Adding (maybe existing) peer with id {} to {topic}.", peer.node_id

	/// This should not start iroh, because receiving a NodeAddr does not mean you want to participate.
	/// This does not start Iroh, because receiving a NodeAddr does not mean you want to participate.

Uh oh!

fix: realtime late join #6869

Are you sure you want to change the base?

fix: realtime late join #6869

Conversation

Septias commented May 21, 2025

Uh oh!

WofWca commented May 22, 2025

Uh oh!

Septias commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Septias commented Jul 8, 2025

Uh oh!

link2xt commented Jul 9, 2025

Uh oh!

Uh oh!

Septias commented Jul 10, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Septias commented Jul 7, 2025 •

edited

Loading