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

Throw an error instead of crashing app when video source is empty #1478

Merged
merged 1 commit into from
Feb 19, 2019

Conversation

laurent22
Copy link
Contributor

If an undefined source is accidentally loaded in react-native-video a fatal error SIGABRT will happen and crash the application. This is very difficult to debug since there's apparently no relation between this crash and react-native-video. So this change checks if the URI is empty and, if it is, it throws an error, allowing the user to find out early about the issue.

If an undefined source is accidentally loaded in react-native-video a fatal error SIGABRT will happen and crash the application. This is very difficult to debug since there's apparently no relation between this crash and react-native-video. So this change checks if the URI is empty and, if it is, it throws an error, allowing the user to find out early about the issue.

https://cloud.githubusercontent.com/assets/5795227/20283860/08223ba6-aabb-11e6-9fdc-d12d32b3aa9a.png
@cobarx cobarx merged commit 98eb7a3 into TheWidlarzGroup:master Feb 19, 2019
@cobarx
Copy link
Contributor

cobarx commented Feb 19, 2019

I thought about this a little more and I think it makes sense to combine this with #1246 where we handle it in the native code so it's the same as ExoPlayer. I'm going to make this a warning.

beauner69 pushed a commit to beauner69/react-native-video that referenced this pull request Oct 10, 2019
Throw an error instead of crashing app when video source is empty

(rebased from commit 98eb7a3)
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