Skip to content

[PM-39361] fix: Restart card scanner after camera interruption or reopen#2847

Open
fedemkr wants to merge 1 commit into
mainfrom
PM-39361/fix-card-scanner-unresponsive
Open

[PM-39361] fix: Restart card scanner after camera interruption or reopen#2847
fedemkr wants to merge 1 commit into
mainfrom
PM-39361/fix-card-scanner-unresponsive

Conversation

@fedemkr

@fedemkr fedemkr commented Jun 30, 2026

Copy link
Copy Markdown
Member

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-39361

📔 Objective

Fixes a blank camera screen when opening the card scanner after the Add Card view was closed while the scanner was still open, or after the device locks and unlocks while the app is on the Add Card screen.

Changes in CardScannerView.swift:

  • Replace try? startScanning() with explicit error handling; on failure a new
    onScannerUnavailable callback is fired (dispatched after the current SwiftUI
    update pass to avoid re-entrant state mutations).
  • Implement dataScanner(_:becameUnavailableWithError:) in the Coordinator to
    forward runtime camera-interruption events through the same callback.
  • Add @Environment(\.scenePhase) observation in CardScannerWrapperView; when
    the scene becomes .active while scanning is supposed to be running, a
    stop-then-restart cycle is triggered.
  • Retry up to 2 times with a 300 ms delay before auto-dismissing the sheet, so
    the user is never left with a permanently blank screen.

@fedemkr fedemkr added the ai-review Request a Claude code review label Jun 30, 2026
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context t:bug Change Type - Bug labels Jun 30, 2026
@fedemkr fedemkr marked this pull request as ready for review June 30, 2026 21:48
@fedemkr fedemkr requested review from a team and matt-livefront as code owners June 30, 2026 21:48
@fedemkr fedemkr added the enhancement New feature or request label Jun 30, 2026
@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: REQUEST CHANGES

Reviewed the card scanner restart logic in CardScannerView.swift and its new unit tests. The change adds explicit error handling around startScanning(), a becameUnavailableWithError delegate forward, scene-phase observation, and a retry-then-dismiss flow. The error-forwarding and dispatch-after-update-pass approach are sound. One correctness concern remains around how the retry budget is shared between failure and scene-phase restarts.

Code Review Details
  • ⚠️ : Benign foreground transitions consume the retry budget and can auto-dismiss a working scanner; counter never resets on a successful restart.
    • BitwardenShared/UI/Vault/VaultItem/AddEditItem/AddEditCardItem/CardScannerView.swift:65-87

Comment on lines +65 to 87
.onChange(of: scenePhase) { newPhase in
if newPhase == .active, isScanning {
restartScanning()
}
}
}

// MARK: Private

/// Stops scanning, waits 300 ms for the camera to fully release, then restarts.
/// After two retries the sheet is dismissed so the user is never left with a blank screen.
private func restartScanning() {
guard scannerRetryCount < 2 else {
dismiss()
return
}
scannerRetryCount += 1
isScanning = false
Task { @MainActor in
try? await Task.sleep(for: .milliseconds(300))
isScanning = true
}
}

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.

⚠️ IMPORTANT: Benign foreground transitions consume the retry budget and can auto-dismiss a working scanner.

Details and fix

scannerRetryCount is shared between two very different triggers: genuine scanner failures (onScannerUnavailable) and routine scenePhase == .active transitions (line 66). It only resets on .onAppear, which does not re-fire when the app is backgrounded and foregrounded while the sheet stays presented.

Trace:

After two normal app-switches the sheet closes on the next foreground even though the camera is fine. A successful restart also never decrements the counter, so each recovery permanently shrinks the budget available for a later real failure.

Consider resetting scannerRetryCount = 0 on a successful restart (e.g. after isScanning = true settles), or only counting onScannerUnavailable-driven restarts toward the cap rather than scene-phase restarts.

do {
try uiViewController.startScanning()
} catch {
DispatchQueue.main.async { context.coordinator.onScannerUnavailable?() }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

❓ Is the dispatch here just to avoid updating the view in the middle of a view update?

Comment on lines +65 to +67
.onChange(of: scenePhase) { newPhase in
if newPhase == .active, isScanning {
restartScanning()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤔 Does the change of scenePhase trigger for you? I think the scene phase might only work if you're using SwiftUI navigation, not UIKit. You might need to observe the notification center notifications instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review app:password-manager Bitwarden Password Manager app context enhancement New feature or request t:bug Change Type - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants