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

Catch race condition while recording voice audio #7009

Merged
merged 1 commit into from
Sep 5, 2022

Conversation

jmartinesp
Copy link
Member

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

Modified AudioMessageHelper to catch possible RuntimeException caused by race conditions while retrieving the audio file.

Motivation and context

See #6989.

Tests

This is difficult to reproduce, to be honest.

  • On a room/DM, tap on the record voice message icon for 1-2s.
  • Repeatedly tap on it again.

With some luck, you'll trigger this race condition.

Tested devices

  • Physical
  • Emulator
  • OS version(s): 11

Checklist

@jmartinesp jmartinesp requested a review from a team September 5, 2022 10:46
@jmartinesp jmartinesp self-assigned this Sep 5, 2022
@jmartinesp jmartinesp requested review from bmarty and removed request for a team September 5, 2022 10:46
@jmartinesp jmartinesp force-pushed the fix/voice-recording-crashes-on-android-10 branch from 5ff7f22 to 526c5ba Compare September 5, 2022 11:42
Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix, LGTM, just one small question.

try {
tryOrNull("Cannot stop media recording amplitude") {
stopRecordingAmplitudes()
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to move this block?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, not really, it was just a test to see if stopping the recording sooner could also help with the race condition and I forgot to revert it.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 5, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@jmartinesp jmartinesp enabled auto-merge (squash) September 5, 2022 15:50
@jmartinesp jmartinesp merged commit ae653eb into develop Sep 5, 2022
@jmartinesp jmartinesp deleted the fix/voice-recording-crashes-on-android-10 branch September 5, 2022 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants