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

[expo-av] Improve error-messages on iOS #9618

Merged
merged 3 commits into from
Aug 17, 2020

Conversation

IjzerenHein
Copy link
Contributor

@IjzerenHein IjzerenHein commented Aug 7, 2020

Why

Not all errors are logged with the actual reason; which makes it more difficult to find the cause.

E.g, when Sound.prepare fails because the app is in the background, this is logged:

prepare encountered an error: audio session not activated!

Which does not include the reason, which is that the app is in the background.

Related to #6294

How

  • Include error-description in error-message
  • Only add error-object when available
  • Re-formatted code

Test Plan

  • All tests succeed

@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2020

Native Component List for this branch is ready

Copy link
Contributor

@bbarthec bbarthec left a comment

Choose a reason for hiding this comment

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

In general LGTM, but I'm not a fan of these error messages 😞
A few of them do not give any meaningful reason for what's happening 🤔
But, don's sweat it as I'm not sure whether those could be improved 😉

packages/expo-av/ios/EXAV/EXAV.m Outdated Show resolved Hide resolved
packages/expo-av/ios/EXAV/EXAV.m Outdated Show resolved Hide resolved
@IjzerenHein IjzerenHein merged commit b717643 into master Aug 17, 2020
@IjzerenHein IjzerenHein deleted the @hein/expo-av/error-logging branch August 17, 2020 09:19
Jamedjo pushed a commit to Jamedjo/expo that referenced this pull request Feb 4, 2021
* [expo-av] Improve error-messages on iOS

* [expo-av] Update changelog

* [expo-av] Improve error messages
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.

None yet

2 participants