[PM-39361] fix: Restart card scanner after camera interruption or reopen#2847
Conversation
github-actions
Bot
added
app:password-manager
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the card scanner restart logic in Code Review DetailsNo new blocking findings. Previously raised concerns are now resolved:
The minor body overlap between |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| .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 | ||
| } | ||
| } |
There was a problem hiding this comment.
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:
- background → foreground Dependency Dashboard #1 →
restartScanning()→scannerRetryCount = 1 - background → foreground re-open Update bitwarden/gh-actions digest to 67ab95d #2 →
scannerRetryCount = 2 - background → foreground Update dependency prettier to v3 #3 → guard fails →
dismiss()
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.
…nner-unresponsive
…annerView Replace scenePhase (unreliable in this UIKit-hosted app) with a UIApplication.willEnterForegroundNotification observer, and stop foreground-triggered restarts from consuming the same retry budget as genuine scanner failures.
| .onChange(of: scenePhase) { newPhase in | ||
| if newPhase == .active, isScanning { | ||
| restartScanning() | ||
| .onReceive(NotificationCenter.default.publisher(for: UIApplication.willEnterForegroundNotification)) { _ in |
There was a problem hiding this comment.
@matt-livefront I wasn't sure whether this would be a good idea to put it directly here. It seems more like SwiftUI but less testable so if needed I can move it to the processor and do the logic somehow there.
There was a problem hiding this comment.
I think it's ok given that we would have used scene phase here and it's for a minor and self-contained use.
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent the card-scanner sheet from getting stuck on a blank camera view after interruptions (e.g., device lock/unlock, leaving/reopening the sheet) by adding explicit scanner-unavailable handling and a controlled restart strategy.
Changes:
- Adds an
onScannerUnavailablecallback that is triggered whenstartScanning()throws or when VisionKit reports the scanner becoming unavailable at runtime. - Implements a stop→delay→restart cycle with a capped retry budget, plus a foreground-triggered restart path.
- Adds unit tests for the coordinator’s forwarding of
becameUnavailableWithErrorto the new callback.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| BitwardenShared/UI/Vault/VaultItem/AddEditItem/AddEditCardItem/CardScannerView.swift | Adds scanner-unavailable callback wiring, restart/retry logic, and foreground resumption handling. |
| BitwardenShared/UI/Vault/VaultItem/AddEditItem/AddEditCardItem/CardScannerViewTests.swift | Adds tests ensuring runtime-unavailable delegate events trigger onScannerUnavailable. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| scannerRetryCount += 1 | ||
| isScanning = false | ||
| Task { @MainActor in | ||
| try? await Task.sleep(for: .milliseconds(300)) | ||
| isScanning = true | ||
| } |
| .onReceive(NotificationCenter.default.publisher(for: UIApplication.willEnterForegroundNotification)) { _ in | ||
| // `scenePhase` is driven by SwiftUI's own `Scene`/`App` lifecycle and is unreliable | ||
| // for views hosted in a UIKit app via `UIHostingController`, so foreground transitions | ||
| // are observed directly via `UIApplication` notifications instead. | ||
| if isScanning { |
| /// 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 |
|
Warning @fedemkr Uploading code coverage report failed. Please check the "Upload to codecov.io" step of Process Test Reports job for more details. |
🎟️ 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:try? startScanning()with explicit error handling; on failure a newonScannerUnavailablecallback is fired (dispatched after the current SwiftUIupdate pass to avoid re-entrant state mutations).
dataScanner(_:becameUnavailableWithError:)in theCoordinatortoforward runtime camera-interruption events through the same callback.
@Environment(\.scenePhase)observation inCardScannerWrapperView; whenthe scene becomes
.activewhile scanning is supposed to be running, astop-then-restart cycle is triggered.
the user is never left with a permanently blank screen.