react-native-audio-waveform
react-native-audio-waveform copied to clipboard
Android : fix multiple leaking promises + bonuses
trafficstars
The Android code has multiple places without a guarantee of resolving the promise. This PR provides numerous fixes for issues I encountered while playing audio in the list. Most of them are just simple code review refactoring.
For example, just opening one audio file made three promise hangs, and each call to seekToPlayer leaks. So, just clicking 500 times on the waveform triggers the below error:
Excessive number of pending callbacks: 501. Some pending callbacks that might have leaked by never being called from native code: {"55":{"module":"AudioWaveform","method":"setPlaybackSpeed"},"89":{"module":"AudioWaveform","method":"setPlaybackSpeed"}
Highlights:
- Keeping promises inside
AudioWaveformModulehelps to ensure promises are resolved correctly - Moving promises out of
AudioPlayerfor a leaner class and not "react-native" tainted - Doing try-catch so the app does not crash when having an exception
Bonus:
- I added a feature in the example to list recorded audio after the app reboot!
- I removed the deprecation of
onPlayerStateChangedin favour ofonPlaybackStateChanged - I reviewed the weird handling of
onErrorand simplified it - I removed a few
Promise.resolveandPromise.rejectcombined with try-catch that are useless and can be simplified by returning the value directly or throwing an error since the method is alreadyasync - I fixed a case where the promise leaks when the path in the
preparePlayeris wrong. If we play a file with an invalid path, the player never gets ready and gets stuck idle. Sinceplayer.prepare()should move it out ofidle, if it stays inidle, there is a problem, and the promise is rejected. - I set props of
IStartPlayermandatory else; I test it, and passing no value crashes the android, so there are no optional - I removed the
await preparePlayerForPath();inside theonDidFinishPlayingAudiosince this one is circular, where you trigger the player's end but recreate it immediately. Also, after removal, it still works - Code simplification by using standard functions to validate the player key string and the audio player in the hash map
Let me know what you think!
Note: I may need to remove my logger logPromise tracking the leaking promises before you merge!