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

checks: Guard float/int conversions #158

Merged
merged 1 commit into from
Jun 8, 2024
Merged

Conversation

Penwy
Copy link
Contributor

@Penwy Penwy commented May 8, 2024

Description

This guards all string to float/int conversions in the analyzer inside try-except clauses, to ensure that it fails gracefully if the string fed to it is not a valid float/int.

Motivation and Context

Due to the ever-changing nature of obs logging, especially with localisation and third-party plugins, it cannot be guaranteed and future-proofed that a given search or regexp will always fetch the bit of the log that it is supposed to.
If, as a result of this, the analyzer calls int() or float() on a string that is not a valid number, it will cause an exception and abort the whole analysis.
For example, as a result of the french localisation adding an unexpected set of parentheses in the name of the replay buffer, this log fails the whole analysis when trying to fetch the amount of lagged frames.

This solves those instances by catching such exceptions, and allowing the rest of the analysis to proceed.

The checkStreamSettings functions haven't been modified, as they are currently not working, and #157 is already set to either modify them with an already guarded alternative, or remove them entirely.

How Has This Been Tested?

Manually fed the log above to the analyzer, checked that it worked well.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@prgmitchell
Copy link
Member

At a glance this seems fine but I am not seeing any commit message.

@Penwy
Copy link
Contributor Author

Penwy commented May 18, 2024

I did not think it warranted one, does it?

@prgmitchell
Copy link
Member

I was under the impression that all commits should have a message but maybe I am mistaken, @RytoEX could confirm.

@RytoEX
Copy link
Member

RytoEX commented May 18, 2024

There is a commit message:

checks: Guard float/int conversions

Not all commits require further elaboration in a commit message body. That said, further elaboration pointing out that sometimes these values may be text strings that cannot be converted, or rephrasing the PR description/motivation, would not be wholly unwelcome.

@prgmitchell
Copy link
Member

Yes sorry, I had my terminology wrong and was referring to a commit message body.

Converting a string to float/int will raise an exception if said string
is not a valid float/int. This makes sure such exceptions are caught
and do not interrupt the rest of the analysis.
@Penwy
Copy link
Contributor Author

Penwy commented May 19, 2024

Done, do tell if you'd want it worded differently.

@RytoEX RytoEX merged commit 077c1a4 into obsproject:master Jun 8, 2024
2 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