Crash: when selecting a title of a featured episode

[pocketcasts]

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:

https://user-images.githubusercontent.com/16253818/174629866-95653d86-f5a6-498b-bf6a-762506228b97.mp4

App Version: 7.20
Device: iPhone14,5
OS: 15.6

#5306175-zen

[jgcaruso]

I believe I've found the crash data

podcasts/EpisodeDetailViewController.swift:118: Fatal error: Unexpectedly found nil while unwrapping an Optional value

Crashed: com.apple.main-thread
0  libswiftCore.dylib             0x3a8c8 closure #1 in closure #1 in closure #1 in _assertionFailure(_:_:file:line:flags:) + 360
1  libswiftCore.dylib             0x3a62c closure #1 in closure #1 in _assertionFailure(_:_:file:line:flags:) + 196
2  libswiftCore.dylib             0x3a434 closure #1 in _assertionFailure(_:_:file:line:flags:) + 208
3  libswiftCore.dylib             0x39f7c _assertionFailure(_:_:file:line:flags:) + 232
4  podcasts                       0x3801ac EpisodeDetailViewController.init(episodeUuid:podcast:) + 118 (EpisodeDetailViewController.swift:118)
5  podcasts                       0x37fb8c EpisodeDetailViewController.__allocating_init(episodeUuid:podcast:) + 117 (EpisodeDetailViewController.swift:117)
6  podcasts                       0x2110cc DiscoverViewController.show(discoverEpisode:podcast:) + 104 (DiscoverViewController+DiscoverDelegate.swift:104)
7  podcasts                       0x211224 protocol witness for DiscoverDelegate.show(discoverEpisode:podcast:) in conformance DiscoverViewController + 4334604836 (<compiler-generated>:4334604836)
8  podcasts                       0x3fa9c8 closure #1 in DiscoverEpisodeViewModel.didSelectEpisode() + 131 (DiscoverEpisodeViewModel.swift:131)
9  Combine                        0xb4b0 Subscribers.Sink.receive(_:) + 96
10 Combine                        0xada8 protocol witness for Subscriber.receive(_:) in conformance Subscribers.Sink<A, B> + 24
11 Combine                        0x129fc closure #1 in Publishers.ReceiveOn.Inner.receive(_:) + 288
12 libswiftFoundation.dylib       0x28af0 <redacted> + 36
13 CoreFoundation                 0x725a4 <redacted> + 28
14 CoreFoundation                 0x73500 <redacted> + 412
15 CoreFoundation                 0xb070 <redacted> + 848
16 CoreFoundation                 0x1ebc8 CFRunLoopRunSpecific + 600
17 GraphicsServices               0x1374 GSEventRunModal + 164
18 UIKitCore                      0x514ee8 <redacted> + 1100
19 UIKitCore                      0x296584 UIApplicationMain + 364
20 podcasts                       0x4345e0 main + 11 (AppDelegate.swift:11)
21 ???                            0x10185dda4 (Missing)

And the code that caused it:

https://github.com/Automattic/pocket-casts-ios/blob/e77eb26e7c39adbb933e7cca848faaf3593344bb/main/podcasts/EpisodeDetailViewController.swift#L117-L122

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.

[jgcaruso]

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:

1. if the episode is missing, show a message that the episode wasn’t found and maybe ask them to try a sync (optimistic but not certain that a sync will solve the issue)
2. if the episode is missing, force a sync to try to download it (not ideal if offline at the time the episode is opened, that will just fail anyways, no guarantee a sync will solve it either)
3. find why the episode wasn’t downloaded in the first place and fix that (will need some more investigation, but so far haven’t been able to find anything)

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”

[leandroalonso]

@jgcaruso most crashes seems to originate from DiscoverViewController+DiscoverDelegate.swift. However we still have issues from ListeningHistoryViewController+Table.swift and DownloadsViewController+Table.swift.

For the DiscoverViewController it seems a sync wouldn't help anyway — and this is the place most crashes comes from. I wonder if any database race conditions might be happening. 🤔

Were you able to reproduce the crash? I'm thinking that before coming up with any solution we can at least check DiscoverViewController just to have an idea of what might be happening. One question I have is: can an episode appear without being on the database? (thus causing the crash)

[jgcaruso]

I haven't been able to reproduce it yet. I did notice the ListeningHistoryViewController+Table.swift crash logs as well, but that one made even less sense to me because in order to listen to the episode it must have existed. I tried a few things to try to listen and then delete an episode, but it does then disappear from the history view. But I can keep testing around this and DownloadsViewController as well to see if there is a way to reproduce it.

One question I have is: can an episode appear without being on the database? (thus causing the crash)

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.

[leandroalonso]

@jgcaruso I think I have some more info to share regarding this issue. About my own question:

One question I have is: can an episode appear without being on the database? (thus causing the crash)

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:

1. A user opens Discover
2. At a point in time, the cleanup happens
3. This clean remove the podcast and the episodes
4. If a user interacts with an episode (leading to the method you shared) the crash will happen

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: https://github.com/Automattic/pocket-casts-ios/pull/137

I think there might be a few things we can do:

1. Do nothing, but for the user that might be weird
2. If this is coming from Discover, we can just force a refresh in the Discover and this should fix it
3. We can somehow avoid deleting episodes if they're shown somewhere, but this can be tricky

Let me know if that makes sense to you and what do you think about the suggested approaches.

[jgcaruso]

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:

1. I agree that this might be weird. At the very least, maybe we can display an Alert so there is some feedback from the tap and it doesn't feel like the app is broken?
2. Forcing a refresh could work if the user has internet access, but if they are offline the app will still crash. But at the same time if you're offline you won't be able to do much with the episodes once you see them since they won't be downloaded. In any case, some kind of feedback on screen would be nice anyways.
3. I agree this can be tricky since there are a few different locations where this crash is happening, and they probably have their own rules. And anywhere they may be added in the future, someone would have to remember to update the delete logic to support that as well.

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 😄

[jgcaruso]

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 https://github.com/Automattic/pocket-casts-ios/pull/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.

https://github.com/Automattic/pocket-casts-ios/blob/8125fbec829839641528e55ac9b00d2523067dc3/podcasts/PodcastManager%2BCleanup.swift#L11-L12

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) https://github.com/Automattic/pocket-casts-ios/blob/e77eb26e7c39adbb933e7cca848faaf3593344bb/podcasts/DiscoverEpisodeViewModel.swift#L140-L142

ServerPodcastManager.shared.updateLatestEpisodeInfo(podcast: existingPodcast, setDefaults: true)

[emilylaguna]

Cross posting a way to reproduce this issue from the PR: https://github.com/Automattic/pocket-casts-ios/pull/271#issuecomment-1262597141