-
Notifications
You must be signed in to change notification settings - Fork 129
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
Crash: when selecting a title of a featured episode #58
Comments
I believe I've found the crash data
And the code that caused it: pocket-casts-ios/main/podcasts/EpisodeDetailViewController.swift Lines 117 to 122 in e77eb26
Apparently its OK to crash here 😄 The issue looks like somehow the Discover view had an episode but the episode didn't exist on the device in the podcast/episode db. Will need to look into how that could have happened. |
I’ve been looking at this issue, and I have a couple of ideas how I could address it but curious what “should” be done:
I’m leaning towards the first one since it is something that can be done now and would be much better than simply crashing. But will also now mask the issue if it becomes more common and a manual sync doesn’t fix it unless people start to complain. 3 feels like the best choice, but not sure how much time should be spent on this issue. According to Crashlytics “This issue has 345 crash events affecting 203 users” |
@jgcaruso most crashes seems to originate from For the Were you able to reproduce the crash? I'm thinking that before coming up with any solution we can at least check |
I haven't been able to reproduce it yet. I did notice the
I was wondering about this too. DiscoverEpisode seems to be a separate struct from a regular Episode, so they could be coming from different places, but I haven't tracked that down yet either. I haven't had a chance to get familiar with the data layer yet, but maybe now is the time. |
@jgcaruso I think I have some more info to share regarding this issue. About my own question:
The answer is yes. A screen might be already appearing — like Discover — and the podcast and the episodes might disappear from the database. This happens because of the cleanup we run from time to time. A scenario like this is very likely:
I wasn't been able to reproduce it yet because right now we don't have any episode card appearing. But in order to easily reproduce the cleanup follow @emilylaguna steps described here: #137 I think there might be a few things we can do:
Let me know if that makes sense to you and what do you think about the suggested approaches. |
Thanks for that follow up! I was only able to artificially reproduce the error by preventing podcast json from being saved to the DB when Discover is loaded (which probably isn't a likely cause), but a cleanup process causing the issue makes a lot of sense. Especially because it seems from that linked PR that the podcast was deleted, but the episodes stuck around... which supports the scenario the user reported where they were still able to stream the episode if they clicked the play button directly instead of tapping the Podcast title! As for your suggestions:
Maybe some mix of 1 and 2 could work? Attempt a re-sync if online, show a message if offline that it isn't available until you regain internet access? 3 feels more correct though especially for such a main feature like Discover. And in the future, if a developer adds the podcast view controller to a new area and the podcast happens to get deleted, there should probably be some kind of crash prevention on the view controller so we don't run into this messy interaction again. So maybe we do all 3 😄 |
I've tried to reproduce the issue with the assumption that it is a "cleanup" process that is deleting the podcast using the steps from #137 however I am still unable to reproduce the issue. It appears that there is already code that is performing what was suggested above for 1 & 2 (download the Podcast if it is missing, do nothing if offline or episode can't be found). I've debugged through the code and observed the podcast and its episodes being deleted by cleanup, and then the next time I try to open the "featured" episode, it re-downloads the podcast. If I'm offline, tapping the episode just does nothing. Unless the cleanup is happening inbetween the podcast being downloaded and the screen attempting to open (which would be incredibly bad luck) I'm not sure if this is the cause of the crash. Looking through the code again and scrutinizing the cleanup code with the crashes from the ListeningHistory view in mind, it looks like the cleanup process does take into account if the user has "interacted" with the episode. It checks things like if it was archived, if it was downloaded and if it was played which would be true for anything in the ListeningHistory table. It also seems like the episode exists when the table is loaded (in order for it to be visible), but then is deleted between then and when the row is tapped. Again, if a cleanup is running just inbetween those 2 steps, that is incredibly bad luck. But, given that the crash happens for such a small number of users (about 200 users in the last 90 days) it could be that something very rare is in fact happening and causing the crash. The only hunch I currently have that I'm waiting on confirmation of some details around is that the data for the Discover tab is protected by a 1 week check from the day when it was added. pocket-casts-ios/podcasts/PodcastManager+Cleanup.swift Lines 11 to 12 in 8125fbe
So if the Discover tab isn't updated every 7 days, and if the cleanup happens to run after the episode is tapped on the Discover tab it could be in a situation where the episode gets deleted out from under the view that is trying to load it. Feels very unlikely though, and also doesn't explain why the original report said it was reproducible for them. I would expect after the first crash, the next time you attempt to load the podcast it would get re-downloaded and then stick around for another week. Another hunch that would support the above is if the episode didn't exist yet when the podcast list was originally downloaded (maybe the user happened to look at it before it was in the Discover tab). If the list is never updated then the Discover tab is referencing an episode id that doesn't exist in the locally cached episodes table, this crash could happen. If that happens to be the case, this may be solvable with 1 line of code in this method, to refresh the episode list (maybe only if the episode we're interested in doesn't exist) pocket-casts-ios/podcasts/DiscoverEpisodeViewModel.swift Lines 140 to 142 in e77eb26
|
Cross posting a way to reproduce this issue from the PR: #271 (comment) |
This was initially reported to us in the Beta Slack.
We have been unable to replicate it; however, the user can consistently replicate the issue and has sent the crash logs to Firebase. Upon closer look, it seems like the user is on the iOS 15.6 Beta, so we may need to double-check if the issue is iOS-related or something we need to fix.
Screen Recording:
RPReplay_Final1655487059.1.mp4
#5306175-zen
The text was updated successfully, but these errors were encountered: