Skip to content

fix: Handle empty disco#items responses - #2253

Closed
MichaelDim02 wants to merge 1 commit into
profanity-im:masterfrom
MichaelDim02:fix/empty-disco-items
Closed

MichaelDim02 wants to merge 1 commit into
profanity-im:masterfrom
MichaelDim02:fix/empty-disco-items

Conversation

@MichaelDim02

Copy link
Copy Markdown

Connection initialization stalls because of an early return on empty disco#items responses.

I encountered this issue when attempting to add a room as a bookmark, which would report success but no bookmark was actually added, with the logs reporting an empty storage field sent to the server. Apparently the bookmarks table was never initialized, as bookmark_request() was never triggered. I traced this to the early exit on _disco_items_result_handler().

This patch fixes the immediate user-facing issue of adding persistent bookmarks. However, merely calling sv_ev_connection_features_received(); on an empty table may not be a complete compensation for initialization, and whether initialization order is respected needs to be examined. Long term, a shared handler may be more appropriate. I decided to leave that decision up to the maintainers.

  • I ran valgrind when using my new feature

How to test the functionality

  • Connect to a server that returns an empty disco#items response
  • Add a room bookmark using /bookmark add room@conference.example.org autojoin on.
  • Verify that /bookmark list displays the newly added room.
  • Disconnect and reconnect, then verify that the bookmark is retrieved and the room is automatically joined.

Signed-off-by: Michail Konstantinos Dimopoulos <mk@mcdim.xyz>
@jubalh

jubalh commented Nov 10, 2026

Copy link
Copy Markdown
Member

I just started to work on that part of the code and pushed a PR one day before your opened this one. Please take a look at #2251

It's not quite finished yet. I still need to check a couple of things before merging.

@jubalh

jubalh commented Nov 11, 2026

Copy link
Copy Markdown
Member

Thanks for your effort.
After reviewing I see this is addressed in #2251 as well. So I'll close in favour of that PR.

@jubalh jubalh closed this Nov 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants