fix(pubsub): handle subscription response on reconnects (#105)

[leruaa]

Motivation

Closes #105

Solution

• Add a in-flight request for each subscription requests to be dispached on reconnect
• Don't drop server ids to be able to update them when handling subscription responses

PR Checklist

• Added Tests
• Added Documentation
• Breaking changes

[prestwich]

can you explain the reasoning behind this fix? i.e. why do we believe it addresses the issue?

It looks like the main difference is the Inflight added here

[leruaa]

@prestwich Indeed. It seems that on reconnects, if there are no in-flight request, new subscription responses are not handled, see below:

alloy/crates/pubsub/src/service.rs

Line 167 in 4db40b0

PubSubItem::Response(resp) => match self.in_flights.handle_response(resp) {

[prestwich]

I am reluctant to merge this while it contains a memory leak (even if it's not a very serious one for most people). can you test with the current code and dropping server ids?

[leruaa]

@prestwich Yes I just did, see my comment above

[prestwich]

ah simultaneous comments. seems the check in upsert self.local_to_server.contains_left(&local_id) should be self.local_to_sub.contains_left(&local_id) ?

[leruaa]

That make sense, I will test.

[leruaa]

My tests are conclusive, changing local_to_server to local_to_sub is enough to fix reconnect :). I think this PR is ready for review.

[prestwich]

amazing, thanks for working in this. I will give it final review when i get to a laptop (hopefully today)

summary:

• Missing inflight prevented correct handling of the replacement subscription response
• incorrect condition in upsert caused overwriting the channel

[prestwich]

lgtm