Skip to content
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

Add multiple session detection, avoid multiple tabs #8442

Merged
merged 1 commit into from
Feb 18, 2025

Conversation

qstokkink
Copy link
Contributor

@qstokkink qstokkink commented Feb 18, 2025

Fixes #8379

This PR:

  • Adds an /info route to the EventsEndpoint.
  • Fixes multiple tabs getting opened when double-clicking torrents, when an existing session is present. When not adding a torrent, a tab is still opened (otherwise, a remote session could lock you out from connecting to the core).
  • Updates find_api_server to query the events endpoint and return basic session/events info.

@qstokkink qstokkink changed the title WIP: Add multiple session detection, avoid multiple tabs READY: Add multiple session detection, avoid multiple tabs Feb 18, 2025
@qstokkink qstokkink marked this pull request as ready for review February 18, 2025 14:11
Copy link
Member

@egbertbouman egbertbouman left a comment

Choose a reason for hiding this comment

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

Couldn't you just make an (events) API request, after getting the url from server_url (like we do in start_download). Maybe this would have been less complicated.

@qstokkink
Copy link
Contributor Author

@egbertbouman do you mean to /api/events? Assuming so, this was my initial approach as well, and the answer I found is: no, that is in fact more complicated.

All in all, the issue I found is that the events endpoint streams in a response. This is not the same as start_download. It means that we need to piece together the bytes response until we find the double-newline delimiter somewhere in the continuous stream and extract the first element, then close off the connection. It's possible, but requires active draining of a connection into some buffer. To avoid all that stream buffering and token parsing, I just added a new single-shot message, so we can use the new call just like start_download.

Copy link
Member

@egbertbouman egbertbouman left a comment

Choose a reason for hiding this comment

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

Fair enough. I had a go anyway:

async with aiohttp.ClientSession() as session:
    response = session.get(server_url + "/api/events", headers={"X-Api-Key": config.get("api/key")})
    await response.content.readline()
    initial_message = (await response.content.readline()).decode()
    session_count = int(json.loads(initial_message[5:]).get("sessions", 0))

Of course this is without exception handling.

@qstokkink
Copy link
Contributor Author

🤔 interesting code. I took some time to evaluate it, but ultimately I still think the implementation of this PR has a lower chance of scary things happening.

@qstokkink qstokkink changed the title READY: Add multiple session detection, avoid multiple tabs Add multiple session detection, avoid multiple tabs Feb 18, 2025
@qstokkink qstokkink merged commit 1323564 into Tribler:main Feb 18, 2025
7 checks passed
@qstokkink qstokkink deleted the upd_events_tentants branch February 18, 2025 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Adding torrents opens multiple Tribler browser tabs
2 participants