Page MenuHomePhorge

Meet: Improved error handling
ClosedPublic

Authored by machniak on Feb 2 2021, 11:49 AM.
Tags
None
Referenced Files
F11583018: D2203.diff
Thu, Mar 28, 4:23 AM
Unknown Object (File)
Wed, Mar 27, 12:41 AM
Unknown Object (File)
Sun, Mar 10, 11:53 AM
Unknown Object (File)
Mon, Mar 4, 9:37 AM
Unknown Object (File)
Feb 21 2024, 3:57 AM
Unknown Object (File)
Jan 29 2024, 8:34 PM
Unknown Object (File)
Jan 19 2024, 9:04 AM
Unknown Object (File)
Jan 18 2024, 10:52 AM
Subscribers
Restricted Project

Details

Reviewers
mollekopf
Group Reviewers
Restricted Project
Commits
rKe4a2e7060d30: Meet: Improved error handling
Summary
  • Display error message or any error
  • Handler errors when connecting to the OpenVidu session
Test Plan

./phpunit

Diff Detail

Repository
rK kolab
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

machniak created this revision.
mollekopf subscribed.
mollekopf added inline comments.
src/resources/vue/Meet/Room.vue
371

Do we need a room state label for 422 as well above?

This revision now requires changes to proceed.Feb 3 2021, 1:52 PM
src/resources/vue/Meet/Room.vue
452

A session.token seems to be implied by a 'ready' state. Can we somehow loose the token?

src/resources/vue/Meet/Room.vue
371

I don't think so. A 422 response should contain the 3xx code response in the json data. In normal circumstances there will be no 422 response without the 3xx code in json response.

452

No, you can't loose the (main) token. Here we're preventing from calling initSession() again if the token has been already acquired. It's needed because after openvidu connection error we might end up in joinSession() again.

See comment why I'm not entirely clear why we don't require the roomStateLabel but nevertheless require a special case for 422, but looks good enough.

src/resources/vue/Meet/Room.vue
371

I don't follow. It seems to me this.roomState can be equal to 422, and in this case it seems like there should be a corresponding label, unless the label is somehow not displayed, in which case I don't get why we need to make an exception for 422.

If you are certain it's correct, that's fine by me.

This revision is now accepted and ready to land.Feb 3 2021, 2:23 PM

I'll review this again regarding 422 handling.

  • Remove redundant condition
This revision is now accepted and ready to land.Feb 3 2021, 3:21 PM
This revision was automatically updated to reflect the committed changes.