Skip to content

VideoFeed Crash When Opened From Multiple Tabs With Slow Network - #2961

Merged
scottkicks merged 2 commits into
mainfrom
scott/video-feed-crash-fix
Aug 18, 2026
Merged

VideoFeed Crash When Opened From Multiple Tabs With Slow Network#2961
scottkicks merged 2 commits into
mainfrom
scott/video-feed-crash-fix

Conversation

@scottkicks

Copy link
Copy Markdown
Contributor

📲 What

Guard against a crash in VideoFeedBannerPresenting when VideoFeedViewController is presented twice in quick succession.

Crash Logs: Affecting a very minor amount of users, but might as well get ahead of it.

🤔 Why

On slow connections, a user could tap "Try it Now" on the Discovery tab, switch to Search while it's still loading, and tap "Try it Now" there too.
Both onReadyToPresent callbacks eventually fire and both attempt to present a VideoFeedViewController through RootTabBarViewController, which can throw an NSInvalidArgumentException and crashes.

🛠 How

In onReadyToPresent, added a guard that checks feedVC.presentingViewController == nil and !feedVC.isBeingPresented before calling present.
Also tightened the existing reuse path guard to use presentingViewController instead of only isBeingPresented, since isBeingPresented returns false once the presentation animation completes.

👀 See

No crash now 🪄

✅ Acceptance criteria

  • Tap "Try it Now" on Discovery on a slow connection (Network Link Conditioner helps)
  • While the banner is loading, switch to Search and tap "Try it Now"
  • App should not crash — second tap is silently ignored
  • Tapping "Try it Now" under normal conditions still opens the feed as expected

@scottkicks
scottkicks marked this pull request as ready for review August 12, 2026 15:04
@scottkicks
scottkicks requested review from a team and stevestreza-ksr and removed request for a team August 12, 2026 15:04
@scottkicks scottkicks added the bug label Aug 12, 2026

@stevestreza-ksr stevestreza-ksr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Makes sense to me. Maybe add an assertion failure there for debugging purposes if this happens, because if something makes this fail in development, it will fail silently.

guard let self, let feedVC else { return }

/// Prevent double-tap and cross tab race condition from presenting the same VC more than once.
guard feedVC.presentingViewController == nil, !feedVC.isBeingPresented else {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You're calling cell?.setLoading(false) twice in the happy path, you could just move that above the guard.

@scottkicks
scottkicks force-pushed the scott/video-feed-crash-fix branch from bc783e6 to 35ef92c Compare August 18, 2026 13:58
@scottkicks
scottkicks merged commit 496997e into main Aug 18, 2026
7 checks passed
@scottkicks
scottkicks deleted the scott/video-feed-crash-fix branch August 18, 2026 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants