Skip to content
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

fix(vMix): fragmented message handling SOFIE-2932 #320

Conversation

ianshade
Copy link
Contributor

About the Contributor

This PR is being opened on behalf of TV 2 Norge.

Type of Contribution

This is a:

Bug fix / Code improvement

Current Behavior

It could happen that incomplete XML message would be attempted to be processed by the 'data' event handler, causing an error to be thrown. As a result the old, incomplete data got indefinitely stuck in the _unprocessedLines of the BaseConnection.

New Behavior

The logic for processing incoming data is extracted, refactored and covered by unit tests.
Handling of messages that may be fragmented over multiple TCP packets is fixed.
Errors thrown during the message handler are no longer interrupting the processing.
When reconnecting, previous and possibly incomplete data is discarded.

Testing Instructions

Other Information

An assumption about the XML message being always terminated by a newline was made. This is not explicitly specified in the documentation, but the documentation says the following:

All messages are UTF8 encoded and \r\n terminated unless specified otherwise.

Optional binary data is the exact length specified in <status/length> including additional terminating characters such as \r\n
which are always appended to the end of multiline text such as from XMLTEXT.

(source: https://www.vmix.com/help25/index.htm?TCPAPI.html)

Status

  • PR is ready to be reviewed.
  • The functionality has been tested by the author.
  • Relevant unit tests has been added / updated.
  • Relevant documentation (code comments, system documentation) has been added / updated.

@codecov-commenter
Copy link

codecov-commenter commented Jan 29, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (d7caf7d) 37.19% compared to head (aa65001) 37.60%.

Files Patch % Lines
...state-resolver/src/integrations/vmix/connection.ts 83.33% 1 Missing ⚠️
...line-state-resolver/src/integrations/vmix/index.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##           release50     #320      +/-   ##
=============================================
+ Coverage      37.19%   37.60%   +0.41%     
=============================================
  Files            105      106       +1     
  Lines          10220    10234      +14     
  Branches        2493     2496       +3     
=============================================
+ Hits            3801     3849      +48     
+ Misses          5868     5837      -31     
+ Partials         551      548       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Julusian
Copy link
Member

Hello!

Thank you for contributing to the Sofie Project!

If you haven’t already, please give our contribution guidelines a read. We will reach out to you if we have any questions on this.

@Julusian Julusian requested a review from a team January 29, 2024 14:37
@Julusian Julusian changed the title fix(vMix): fragmented message handling fix(vMix): fragmented message handling SOFIE-2932 Feb 2, 2024
@Julusian Julusian merged commit c8fe3b1 into nrkno:release50 Feb 2, 2024
17 of 18 checks passed
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.

3 participants