Skip to content

Conversation

jevolk
Copy link

@jevolk jevolk commented Sep 25, 2025

This change lifts the restriction on servers provided with federation alias resolution by providing the same servers already returned with client alias resolution.

The problem is that users favor joining rooms via coveted matrix.org aliases which are the most reliably resolved yet the least reliably joined. The abridged list of servers in the above case often yields a cloudflare 524 customer-timeout.

An attempt was made to allay the blame placed upon us from users suffering join failures by shuffling matrix.org to the back of the servers list. This solution is ineffective with servers lists abridged to a single element.

If this patch is declined the server will have no choice but to fallback to resolving aliases via the client-server endpoint to obtain the unabridged list; obviously not intended and easily preventable with this patch.

If the disposition is to accept I can look into providing the changelog and any additional signoffs as requested.

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Code style is correct (run the linters)

@jevolk jevolk requested a review from a team as a code owner September 25, 2025 05:24
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Contributor

@reivilibre reivilibre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(please hit re-request afterwards; I know this isn't really a 'review' so much as a question)

"Room alias %r not found" % (room_alias.to_string(),),
Codes.NOT_FOUND,
)
return await self.get_association(room_alias)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Afraid I'm not following how this improves anything, is there any chance you can sketch an example?

To put my confusion into words:

get_association appears to just call get_association_from_room_alias if the alias is managed by this server anyway.
And if the alias is not owned by this server, we make a federation request to the server that does.

But why would someone be sending us an SS API query for an alias that we don't own?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the timely response 🙏🏻. To clarify, our high-level goal here is only to populate the response with extra servers. There is no intent to change the resolution process or answer queries on behalf of other servers.

I sympathize with your confusion as I just revisited get_association() myself just now. The branch entered if the alias is owned by the server to call get_association_from_room_alias() does not return early. This allows extra_servers to be populated further down in get_association() and that's what will achieve the goal here.

@jevolk jevolk requested a review from reivilibre September 26, 2025 23:12
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.

4 participants