Skip to content
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

fix: codeScanner unsupported ios version error handing #3175

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mgefimov
Copy link
Contributor

@mgefimov mgefimov commented Sep 7, 2024

What

Enabling codabar format in code scanner is dangerous

Scenario: You set up the code scanner with several formats, including codabar. On Android and iOS 15.4 and above, everything works well. However, on iOS 15.3 or below, you get a black screen, and the scanner doesn't work at all.

Changes

On iOS 15.3 or below, if you enable an unsupported code scanner format (e.g., codabar), the scanner works without this format, instead of a black screen.

Tested on

  • iphone SE (second rev.), iOS 15.0
  • iphone X, ios 15.5

Related issues

Copy link

vercel bot commented Sep 7, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-vision-camera ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 4, 2024 4:42pm

} catch let CameraError.codeScanner(codeScannerError) {
switch codeScannerError {
case .iosVersionNotSupported:
VisionLogger.log(level: .warning, message: codeScannerError.message)
Copy link
Owner

Choose a reason for hiding this comment

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

There's no point in throwing an error if we just ignore it later.

Please add the ios-version-not-supported error to the TypeScript error types, and then in here we just let the error propagate upwards for the user to handle.

Copy link
Owner

Choose a reason for hiding this comment

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

but then hmm he will just ignore it later anyways as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it okay to propagate the error to the JS side? In that case, the onError callback will fire, and the user can handle the error. However, the user might assume that this callback fired because the camera failed to start and cannot operate, leading them to show an error screen, for instance. But the scanner can still operate.

I can refactor the solution without throwing an error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mrousavy

I've refactored without throwing an error. Also i've added supported code types table to the docs

@mgefimov
Copy link
Contributor Author

mgefimov commented Nov 4, 2024

Hi @mrousavy . I hope you're doing well! I have updated PR, can you review it please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants