fix: added support for Roap optimized flow for offer/answer/ok #3271

marcin-bazyl · 2023-12-19T18:49:00Z

COMPLETES

https://jira-eng-gpk2.cisco.com/jira/browse/SPARK-484740

This pull request addresses

Existing Roap offer/answer/ok flow is slow and prone to errors, because it consists of 2 http requests and 1 websocket message:

http request to Locus with OFFER
websocket message from Mercury with ANSWER
http request to Locus with OK

by making the following changes

Using the "optimized" flow, where we indicate in Roap headers ("includeAnswerInHttpResponse" and "noOkInTransaction") that we want to receive the ANSWER in the http response and we don't want to send any OK message. Server then confirms that they don't expect an OK from us by adding the "noOkInTransaction" header to their ANSWER. This way the whole offer/answer/ok exchange can be done in 1 http request.

related internal-media-core PR: https://sqbu-github.cisco.com/WebExSquared/webrtc-media-core/pull/204

Change Type

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation update
Tooling change
Internal code refactor

The following scenarios where tested

< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >

I certified that

I have read and followed contributing guidelines
I discussed changes with code owners prior to submitting this pull request
I have not skipped any automated checks
All existing and new tests passed
I have updated the documentation accordingly

Make sure to have followed the contributing guidelines before submitting.

aws-amplify-us-east-2 · 2023-12-19T18:52:30Z

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-3271.d3m3l2kee0btzx.amplifyapp.com

marcin-bazyl · 2023-12-22T13:06:43Z

packages/@webex/plugin-meetings/src/meeting/index.ts

+   * @param {RoapMessage} roapMessage roap message
+   * @returns {undefined}
+   */
+  public roapMessageReceived = (roapMessage: RoapMessage) => {


this function is just code moved from meetings util

Is there any reason why we can't now move this into the roap folder and remove roap references (except for the one roap object) from meeting/index?

I'm not convinced if that belongs better in the roap folder. The code in SDK's roap folder now is only dealing with preparing the http requests for Locus when sending roap messages and handling the responses, it doesn't even reference mediaProperties or webrtcMediaConnection anywhere. Meeting object is the one that knows about the media connection it has and the roap messages need to be forwarded to it.
There is also other code related to roap in meeting/index - see inside setupMediaConnectionListeners(). Maybe we could look at moving all of this into roap folder... not sure...

marcin-bazyl · 2023-12-22T13:09:02Z

packages/@webex/plugin-meetings/src/meeting/index.ts

+   *
+   * @returns {undefined}
+   */
+  setupSdpListeners = () => {


the code in these event handlers is moved from roap event handlers. I had to introduce these SDP specific events and handle them here separately from the roap events, because previously SDK was relying on OK message to know that answer was processed successfully, now we might not get Roap OK at all. This is also better overall to decouple the roap sequence from the SDP processing, now SDK code doesn't have any assumptions about how roap works, it just forwards the roap events back and forth between Locus and internal-media-core

torgeadelin

looks good

chrisadubois

This is nice, I like the new abstraction for it. One place would be nice to improve for the future would be for the SDK (in meeting/index) to not have any calls to ROAP at all, such as roapMessageReceived but instead perhaps handle this outside and completely remove roap references.

Other than that, just one small comment/question re: mediaConnections[0]

chrisadubois · 2023-12-22T18:01:21Z

packages/@webex/plugin-meetings/src/roap/index.ts

-          return locus;
+          let roapAnswer;
+
+          if (mediaConnections?.[0]?.remoteSdp) {


Should we do something when mediaConnections[0]?.remoteSDP is undefined?

I think this might be handled elsewhere, so a comment may be nice to have here for the cases when this would be undefined.

I think it would indicate a malformed response from the server in this case at this stage, right?

I think mediaConnections[0]?.remoteSDP is always there, I've added the code to handle it missing just in case, I don't think we should ever get a 2xx response from Locus and that not be there.

chrisadubois · 2023-12-22T18:06:00Z

packages/@webex/plugin-meetings/src/meeting/index.ts

+   * @param {RoapMessage} roapMessage roap message
+   * @returns {undefined}
+   */
+  public roapMessageReceived = (roapMessage: RoapMessage) => {


Is there any reason why we can't now move this into the roap folder and remove roap references (except for the one roap object) from meeting/index?

* feat(meeting-info-fail): destroy meeting on meeting info fail if flag is true (webex#3200) Co-authored-by: ndelmar <ndelmar@cisco.com> * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.262 [skip ci] * fix: added metrics to addMedia success/failure metrics (webex#3220) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.263 [skip ci] * fix(plugin-meetings): screen share when shared by same user different client (webex#3177) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.264 [skip ci] * fix(internal-plugin-metrics): bump event dictionary version (webex#3216) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.265 [skip ci] * feat(plugin-metrics): classify network errors (webex#3224) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.266 [skip ci] * feat(plugin-metrics): classify unauthorized errors (webex#3229) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.267 [skip ci] * fix(public): setcorrelation id is made public to be used in cantina (webex#3230) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.268 [skip ci] * feat(plugin-metrics): use network category for relevant errors (webex#3231) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.269 [skip ci] * fix: added logging for getCurrentConnectionType() error cases (webex#3233) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.270 [skip ci] * fix(plugin-meetings): add latency metrics for addMedia (webex#3234) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.271 [skip ci] * feat(ca): add identifiers confId, mid (webex#3226) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.272 [skip ci] * fix: log call parameters when mercury _emit fails (webex#3228) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.273 [skip ci] * feat(ca): add fallback to preLoginId for userId (webex#3221) Co-authored-by: Catalin <ctorge@cisco.com> * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.274 [skip ci] * chore(internal-media-core): version bump (webex#3237) * docs(api): update docs [skip ci] * fix(plugin-meetings): update internal media core to 2.0.6 (webex#3241) Co-authored-by: Bryce Tham <btham@cisco.com> * docs(api): update docs [skip ci] * fix: downscopeing issues (webex#3211) make sure SDK never request new user token from the server with invalid scope(s) and log every attempt correctly * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.275 [skip ci] * fix(breakouts): user gets removed from meeting after end of session (webex#3239) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.276 [skip ci] * fix(plugin-meetings): audio still working when addMedia called with audioEnabled: false (webex#3247) Co-authored-by: ndelmar <ndelmar@cisco.com> * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.277 [skip ci] * fix(plugin-meetings): add tls transport type (webex#3243) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.278 [skip ci] * feat(media): add ice failure groupings for CA (webex#3240) Co-authored-by: Catalin <ctorge@cisco.com> * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.279 [skip ci] * fix(integration): fix waitForPublished() in SDK integration tests (webex#3242) Co-authored-by: Priya Kesari <pkesari@cisco.com> * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.280 [skip ci] * feat(meeting): stats report on media failure (webex#3245) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.281 [skip ci] * fix(plugin-meetings): refactor addMedia and add separate timeout for sdp exchange (webex#3244) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.282 [skip ci] * fix(samples): avoid init failure for tokens without additional features (webex#3257) * fix(metrics) : Call diagnostics validator (webex#3259) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.283 [skip ci] * fix(metrics): catalog-delay (webex#3235) Co-authored-by: peter-robert-cole <peter7cole@gmail.com> * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.284 [skip ci] * feat(logs): add log metadata (webex#3260) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.285 [skip ci] * fix: local linking with internal-media-core (webex#3265) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.286 [skip ci] * feat(plugin-metrics): payload overrides (webex#3270) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.287 [skip ci] * feat(webinar): add webinar related props (webex#3253) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.288 [skip ci] * fix(calling): exporting interfaces, types from index.ts file (webex#3275) Co-authored-by: Priya Kesari <pkesari@cisco.com> * fix(call): error-handling-for-empty-track-while-answer-dial (webex#3277) Co-authored-by: Shreyas Sharma <shreysh2@cisco.com> * fix: use candidate pairs to identify correct local candidate (webex#3258) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.289 [skip ci] * feat(members): add reclaimHostRole functionality (webex#3272) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.290 [skip ci] * fix(calling): remove logging of personal info (webex#3279) * fix(metrics): update to confIdStr (webex#3280) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.291 [skip ci] * feat(calling): add updateMedia in call (webex#3284) * feat(plugin-meetings): retry media connection on turn tls (webex#3261) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.292 [skip ci] * fix(plugin-meetings): force turn discovery on reconnection (webex#3285) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.293 [skip ci] * Fix mercury log (webex#3236) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.294 [skip ci] * fix: added support for Roap optimized flow for offer/answer/ok (webex#3271) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.295 [skip ci] * fix(calling): extend eventing class on ILine interface (webex#3286) Co-authored-by: Priya Kesari <pkesari@cisco.com> * fix(calling): export LocalMicrophoneStream (webex#3290) Co-authored-by: Priya Kesari <pkesari@cisco.com> * fix(plugin-logger): redact mtid (webex#3287) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.296 [skip ci] * feat(plugin-metrics): add error handling to assign role function (webex#3289) Co-authored-by: ndelmar <ndelmar@cisco.com> * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.297 [skip ci] * feat(plugin-meetings): fix .only test (webex#3294) Co-authored-by: ndelmar <ndelmar@cisco.com> * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.298 [skip ci] * fix(samples): reorder and group the ui with similar features (webex#3262) Co-authored-by: Rajesh Kumar <rajeshtezu90@gmail.com> * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.299 [skip ci] * fix(plugin-meetings): don't send turn latency if it fails (webex#3296) * fix(plugin-meetings): add retriedWithTurnServer to add media success metric (webex#3295) * fix(plugin-meetings): don't wait for timeout when sending roap offer fails (webex#3293) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.300 [skip ci] * fix: update internal-media-core to 2.2.1 (webex#3298) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.301 [skip ci] * feat(http-core): capitalize Authorization header (webex#3300) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.302 [skip ci] * feat(fraud): add localIP to join request (webex#3282) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.303 [skip ci] * fix(meetings): ensure non-undefined value for peripheral information property (webex#3281) * docs(api): update docs [skip ci] * chore(release): v3.0.0-beta.304 [skip ci] * test(metrics): skipped failing tests with todo * fix(samples): merge commit resolution --------- Co-authored-by: Natalia del Mar <56999622+nataliadelmar@users.noreply.github.com> Co-authored-by: ndelmar <ndelmar@cisco.com> Co-authored-by: Webex Publisher <webex-jenkins.gen@cisco.com> Co-authored-by: Marcin <marcin.bazyl@gmail.com> Co-authored-by: shivani1211 <shivaniyadav1211@gmail.com> Co-authored-by: Lisa Smith <82507773+lismith2-cisco@users.noreply.github.com> Co-authored-by: Coread <coread@cisco.com> Co-authored-by: Martin Roos <82158095+martroos@users.noreply.github.com> Co-authored-by: shnaaz <82164376+shnaaz@users.noreply.github.com> Co-authored-by: Jordan Rowan <86778628+jor-row@users.noreply.github.com> Co-authored-by: chrisadubois <chdubois@cisco.com> Co-authored-by: László Vadász <maxinteger@gmail.com> Co-authored-by: Catalin Torge <torgeadelin@gmail.com> Co-authored-by: Catalin <ctorge@cisco.com> Co-authored-by: Bryce Tham <brycetham@users.noreply.github.com> Co-authored-by: Bryce Tham <btham@cisco.com> Co-authored-by: Charles Burkett <chburket@cisco.com> Co-authored-by: Tyler McCarthy <74213828+mccarthytyler@users.noreply.github.com> Co-authored-by: Kesari3008 <65543166+Kesari3008@users.noreply.github.com> Co-authored-by: Priya Kesari <pkesari@cisco.com> Co-authored-by: Sreekanth Narayanan <131740035+sreenara@users.noreply.github.com> Co-authored-by: Timothy Scheuering <InteractiveTimmy@users.noreply.github.com> Co-authored-by: peter-robert-cole <peter7cole@gmail.com> Co-authored-by: JudyZhu <120536178+JudyZhuHz@users.noreply.github.com> Co-authored-by: Shreyas Sharma <72344404+Shreyas281299@users.noreply.github.com> Co-authored-by: Shreyas Sharma <shreysh2@cisco.com> Co-authored-by: Rajesh Kumar <131742425+rarajes2@users.noreply.github.com> Co-authored-by: Rajesh Kumar <rajeshtezu90@gmail.com> Co-authored-by: Bin Xu <71026292+bxu2@users.noreply.github.com>

marcin-bazyl force-pushed the spark-484740 branch from 6b8280a to b82de2f Compare December 21, 2023 19:22

fix: added support for Roap optimized flow for offer/answer/ok

023b924

marcin-bazyl force-pushed the spark-484740 branch from b82de2f to 023b924 Compare December 22, 2023 13:04

marcin-bazyl commented Dec 22, 2023

View reviewed changes

marcin-bazyl marked this pull request as ready for review December 22, 2023 13:11

marcin-bazyl requested review from a team as code owners December 22, 2023 13:11

torgeadelin approved these changes Dec 22, 2023

View reviewed changes

marcin-bazyl added 2 commits December 22, 2023 16:04

fix: added metric when answer does not come in http response

bdbe675

fix: test code formatting

92952ab

chrisadubois approved these changes Dec 22, 2023

View reviewed changes

Merge branch 'beta' into spark-484740

8e0c8f1

marcin-bazyl merged commit 8cf8f57 into webex:beta Jan 4, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: added support for Roap optimized flow for offer/answer/ok #3271

fix: added support for Roap optimized flow for offer/answer/ok #3271

marcin-bazyl commented Dec 19, 2023

aws-amplify-us-east-2 bot commented Dec 19, 2023

marcin-bazyl Dec 22, 2023

chrisadubois Dec 22, 2023

marcin-bazyl Dec 22, 2023

marcin-bazyl Dec 22, 2023

torgeadelin left a comment

chrisadubois left a comment

chrisadubois Dec 22, 2023

chrisadubois Dec 22, 2023

marcin-bazyl Dec 22, 2023

chrisadubois Dec 22, 2023

fix: added support for Roap optimized flow for offer/answer/ok #3271

fix: added support for Roap optimized flow for offer/answer/ok #3271

Conversation

marcin-bazyl commented Dec 19, 2023

COMPLETES

This pull request addresses

by making the following changes

Change Type

The following scenarios where tested

I certified that

aws-amplify-us-east-2 bot commented Dec 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

torgeadelin left a comment

Choose a reason for hiding this comment

chrisadubois left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment