Skip to content

fix(rtc): correct join_response attribute access in FAST reconnect#242

Merged
aliev merged 1 commit into
mainfrom
fix/reconnect-join-response-data
May 12, 2026
Merged

fix(rtc): correct join_response attribute access in FAST reconnect#242
aliev merged 1 commit into
mainfrom
fix/reconnect-join-response-data

Conversation

@aliev
Copy link
Copy Markdown
Member

@aliev aliev commented May 12, 2026

Why

ConnectionManager._connect_internal stores join_response.data as self.join_response, so credentials live at the top level. _reconnect_fast was reading self.join_response.data.credentials.token, which raises AttributeError: 'JoinCallResponse' object has no attribute 'data' the first time a FAST reconnect actually runs.

This codepath was never exercised in production because nothing triggered a reconnect on signaling-WS loss. With #241 finally driving into ReconnectionManager, this latent bug surfaces and would crash the first FAST attempt — so it needs to land before (or together with) that PR.

Changes

  • Drop the spurious .data hop in ReconnectionManager._reconnect_fast (getstream/video/rtc/reconnection.py).

Related: #241

Summary by CodeRabbit

  • Bug Fixes
    • Fixed reconnection flow to retrieve authentication tokens from the correct location in the response, ensuring more reliable connection re-establishment.

Review Change Stack

`ConnectionManager._connect_internal` stores `join_response.data` as
`self.join_response` (see `connection_manager.py:387`), so credentials
live at the top level. `_reconnect_fast` was reading
`self.join_response.data.credentials.token`, which raises
`AttributeError: 'JoinCallResponse' object has no attribute 'data'`
the first time a FAST reconnect actually runs.

This codepath was never exercised before because nothing triggered a
reconnect on signaling-WS loss; with the prior commit on this branch
finally driving into `ReconnectionManager`, the latent bug surfaces.

Drop the spurious `.data` hop.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 12, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e5024262-c80a-4ea6-a2ed-e179470ec4b2

📥 Commits

Reviewing files that changed from the base of the PR and between 8c880c5 and 4bbf8c0.

📒 Files selected for processing (1)
  • getstream/video/rtc/reconnection.py

📝 Walkthrough

Walkthrough

This PR corrects the authentication token source in the fast reconnection flow. The _reconnect_fast method now accesses credentials directly from the top-level credentials field of the join_response instead of from a nested data.credentials path, with clarifying comments added to document the payload structure.

Changes

Fast Reconnection Token Source Correction

Layer / File(s) Summary
Fast reconnection token source update
getstream/video/rtc/reconnection.py
ReconnectionManager._reconnect_fast retrieves the authentication token from join_response.credentials.token (top level) instead of join_response.data.credentials.token. Inline comments document that join_response is already the unwrapped payload and credentials are expected at the top level.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

🐰 A token in the right place at last,
No nested depths to access fast,
Credentials revealed at the top so clear,
Comments show the path sincere!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main fix: correcting how join_response attributes are accessed in the FAST reconnect logic by removing the unnecessary .data hop.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/reconnect-join-response-data

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@aliev aliev marked this pull request as ready for review May 12, 2026 10:23
@aliev aliev merged commit c71b551 into main May 12, 2026
21 checks passed
@aliev aliev deleted the fix/reconnect-join-response-data branch May 12, 2026 10:49
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