Skip to content

fix: guard room ConnectionState nil engine - #978

Open
mariusgassen wants to merge 1 commit into
livekit:mainfrom
mariusgassen:features/mariusgassen/engine-guard
Open

fix: guard room ConnectionState nil engine#978
mariusgassen wants to merge 1 commit into
livekit:mainfrom
mariusgassen:features/mariusgassen/engine-guard

Conversation

@mariusgassen

Copy link
Copy Markdown
  • with the refactoring of reading the connection state of a room directly from its non-exposed engine, that will panic if nil when the connection state is read
  • Current workaround: Check for an exposed LocalParticipant

…e is nil

* with the refactoring of reading the connection state of a room directly from its non-exposed `engine`, that will panic if `nil` when the connection state is read
* Current workaround: Check for an exposed `LocalParticipant`
@mariusgassen
mariusgassen requested a review from a team as a code owner August 25, 2026 09:47

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@mariusgassen mariusgassen changed the title fix: return ConnectionStateDisconnected room ConnectionState if engin… fix: guard room ConnectionState nil engine Aug 25, 2026
Comment thread room.go
r.lock.RLock()
defer r.lock.RUnlock()

if r.engine == nil {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

r.engine is initiated in NewRoom and has never been cleared/reassign, it should not be nil. Could you share the panic stack?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

We wrapped the room into a custom struct with business logic on it.

In a test we instantiate it with a &lksdk.Room{} instead of using NewRoom - this will cause the panic because the engine is never initialized.

I fixed our tests by using NewRoom(nil), however I thought a nil guard wouldn't hurt.

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