-
Notifications
You must be signed in to change notification settings - Fork 105
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
Error Handling: gracefully handle missing livekit service url in config #3027
Conversation
9a0ba05
to
e6e2143
Compare
e6e2143
to
0989e04
Compare
0989e04
to
3c23e5a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One material thing to look at:
- wording for ConnectionLostError
Otherwise just nits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking good to me 👍
As we are using existing strings for the "connection lost" description I think leave the wording as is.
Fixes #2971
How to test:
config.json
inpublic/
folder and remove thelivekit
entryWill generate this screen:
I tried to put a clear and not technical message, with a call to action to contact the server admin.
The
error code
should be enough for the admin to know what to doI tried to do a more complete test that would setup a lobby with a video preview and trigger this error, but I would need more time to do so. Any direction could help.
Update
Following internal discussions did a quick refactoring to improve error structure (add a category to match this document), and also added more details on the RTC focus error: Include the domain and better title/description.
See this commit: 2ba803f
Now Error is:
