App crashes when entering exploration with voiceover data without internet connectivity #1340
Assignees
Labels
Priority: Essential
This work item must be completed for its milestone.
Z-ibt
Temporary label for Ben to keep track of issues he's triaged.
Milestone
Comments
BenHenning
added
Priority: Essential
|
Note: this issue also affects images: It's a bit easier to hit this when caching is enabled. |
|
Ah, it actually seems Glide doesn't crash. Only the audio player is triggering a crash, but images just don't seem to load (as expected). |
|
This crash is not having in the latest develop branch. |
|
I think this will only repro if you enable local file caching via . |
BenHenning
added a commit
that referenced
this issue
Aug 13, 2020
This does a bunch of refactoring & hacky workarounds to keep asset priming support fully isolated (just a few files now need to be deleted to clean things up). This also: 1. Fixes actual asset downloading (the GCS asset path templates have changed, so priming didn't actually work anymore). 2. Disables audio file caching since we can't play audio when offline, anyway (there's a dialog that prevents this in-player). 3. Fixes #1340 and #1341 by accounting for error cases when trying to play audio. It turns out that playing audio crashes if you didn't have internet access when going into an exploration (the no connectivity dialog only appears if you lose connectivity within a lesson). There's also some issues with existing view model code that wrongly assumed audio couldn't be in a failure state at that point. This has been fixed. 4. Moves the list of topics to cache to be in the same position as the flag enabling/disabling this functionality. The download experience isn't perfect, but it's meant to be a helper for user study coordinators so that they know when lessons can be brought offline. Note that the UI aspects of this change are really hacky. This is by design--I didn't want to overcomplicate the solution, and I wanted to keep the priming changes fully isolated to make future cleanup easier. Finally, no new tests were added. I clarified in StateFragmentTest that the edge cases fixed in this PR need to have corresponding tests, but I'm actually not sure offhand how to test that audio's playing. I think this will require additional work. I prefer to push this off to #388, but I will follow up with tests if anyone wants to push back on this.
|
It seems the issue here is that the view model assumes that there's no failure in some circumstances, and uses |
BenHenning
added a commit
that referenced
this issue
Aug 17, 2020
…line support (#1636) * Show a dialog when preloading assets for offline support. This does a bunch of refactoring & hacky workarounds to keep asset priming support fully isolated (just a few files now need to be deleted to clean things up). This also: 1. Fixes actual asset downloading (the GCS asset path templates have changed, so priming didn't actually work anymore). 2. Disables audio file caching since we can't play audio when offline, anyway (there's a dialog that prevents this in-player). 3. Fixes #1340 and #1341 by accounting for error cases when trying to play audio. It turns out that playing audio crashes if you didn't have internet access when going into an exploration (the no connectivity dialog only appears if you lose connectivity within a lesson). There's also some issues with existing view model code that wrongly assumed audio couldn't be in a failure state at that point. This has been fixed. 4. Moves the list of topics to cache to be in the same position as the flag enabling/disabling this functionality. The download experience isn't perfect, but it's meant to be a helper for user study coordinators so that they know when lessons can be brought offline. Note that the UI aspects of this change are really hacky. This is by design--I didn't want to overcomplicate the solution, and I wanted to keep the priming changes fully isolated to make future cleanup easier. Finally, no new tests were added. I clarified in StateFragmentTest that the edge cases fixed in this PR need to have corresponding tests, but I'm actually not sure offhand how to test that audio's playing. I think this will require additional work. I prefer to push this off to #388, but I will follow up with tests if anyone wants to push back on this. * Lint fixes. * Introduce new caching module to simplify tests, and fix app module tests. * Lint fixes.
BenHenning
moved this to Needs Triage
in [Team] Core Learner and Mastery flows & UI Frontend - Android
BenHenning
added
the
Z-ibt
seanlip
moved this from Needs Triage
to Done
in [Team] Core Learner and Mastery flows & UI Frontend - Android
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
The app crashes when opening an exploration that has audio voiceovers (e.g. "What is a Fraction?") when not connected to the internet.
To Reproduce
Steps to reproduce the behavior:
Expected behavior
App should not crash.
Screenshots
Device
developAdditional context
Here's the observed stack trace:
It seems that although the media for audio voiceovers are being loaded on a background thread, that thread isn't resilient to exceptions being thrown. While we can probably make this more robust, this seems like a regression: we shouldn't be loading audio right when the exploration is opened. It should only be loaded when we start playing audio, and that should be guarded by an internet connectivity check.
The text was updated successfully, but these errors were encountered: