App
App copied to clipboard
[$500] Camera doesn't load when trying to smartscan a receipt
If you havenβt already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 1.4.38-1 Reproducible in staging?: y Reproducible in production?: y If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: @rlinoz Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1707336890268789
Action Performed:
- Tap request money
- Press the round button to take a picture
- Tap the back arrow
- Again press the Button to take a picture
Expected Result:
Able to scan the receipt again
Actual Result:
Camera doesn't load when trying to smartscan
Workaround:
unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
- [x] Android: Native
- [ ] Android: mWeb Chrome
- [ ] iOS: Native
- [ ] iOS: mWeb Safari
- [ ] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
Add any screenshot/video evidence
https://github.com/Expensify/App/assets/38435837/9f09590f-fbcf-4c86-a4e2-0a300f3c9856
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~0176ddbc1361cf082e
- Upwork Job ID: 1755378138365923328
- Last Price Increase: 2024-02-07
Job added to Upwork: https://www.upwork.com/jobs/~0176ddbc1361cf082e
Triggered auto assignment to Contributor-plus team member for initial proposal review - @s77rt (External
)
Triggered auto assignment to @garrettmknight (Bug
), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
Proposal
Please re-state the problem that we are trying to solve in this issue.
Camera fails to load after navigating away and then returning to it
What is the root cause of that problem?
When user captures the photo first time and navigate to next screen, camera is set inactive here.
https://github.com/Expensify/App/blob/9f37d3e2c1264420773c886ae6171748e5d512b8/src/pages/iou/request/step/IOURequestStepScan/NavigationAwareCamera/index.native.js#L13-L23
This causes the camera library to disable camera when users navigates away from TabNavigator. On refocus the video capture is re-enabled. However it throws below error.
DEBUG [warn] Error taking photo - {"name":"unknown/unknown","_code":"unknown/unknown","_message":"[unknown/unknown] Not bound to a valid Camera [ImageCapture:androidx.camera.core.ImageCapture-eab74056-9b89-4653-8288-e44bc9b65a66]"}
Though camera is enabled on refocus, photo capture fails. This looks like a bug in react-native-vision camera.
What changes do you think we should make in order to solve the problem?
Looks like a fix is available upstream https://github.com/mrousavy/react-native-vision-camera/pull/2339. But it is available only in release v3.70. We've pinned react-native-vision-camera
at 2.16.5
https://github.com/Expensify/App/blob/9f37d3e2c1264420773c886ae6171748e5d512b8/package.json#L164
So one option is to upgrade to v3.7
which could be a major undertaking.
Instead what I found is that the camera gets re-enabled if we toggle the photo prop of Camera
on refocus. This enables photo capture capabilities of camera on refocus and above error is not thrown any more.
const isCameraActive = useTabNavigatorFocus({tabIndex: cameraTabIndex});
return (
<Camera
ref={ref}
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
isActive={isCameraActive}
// Toggle the photo prop as well
photo={isCameraActive}
/>
);
Result
https://github.com/Expensify/App/assets/6880914/7a5007bf-ebfe-43aa-a61b-017dd4d94144
What alternative solutions did you explore? (Optional)
None
Asked in thread here. Is this a deploy blocker if on staging only, or a fire if it's already on prod? It's a core flow, so if the camera can't load to take a receipt that's pretty bad!
@aswin-s no we cannot simply update to v3, there are some feature missing unfortunately, like flash.
@janicduplessis @ishpaul777 any chance this could be stemming from this update? i see the camera worked for you both in the PR, not sure clearly ifyou have gone back and again to the camera, I would assume yes https://github.com/Expensify/App/pull/35014
@mountiny I've detailed how we can fix this without a version upgrade
We are looking into it whether we can find a better solution than the proposed workaround, give us a moment please!
~~(longterm we should look into upgrading V3!)~~ Tracking issue for that is here
@janicduplessis @ishpaul777 any chance this could be stemming from this update?
It appears that the root cause is stemming from the react-native-vision-camera upstream, so i dont think this comes from autofocus PR, Also this is hard to reproduce (at least on my device) Camera turns active before i get the chance to click the capture button.
I can reproduce it on Samsung M 52. Build 1.4.38-2
https://github.com/Expensify/App/assets/115492554/96d5f2d2-b1c4-431d-abb6-954424797ea6
:wave: Friendly reminder that deploy blockers are time-sensitive β± issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:
- Identify the pull request that introduced this issue and revert it.
- Find someone who can quickly fix the issue.
- Fix the issue yourself.
Triggered auto assignment to @youssef-lr (Engineering
), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.
Production -not reproduced
https://github.com/Expensify/App/assets/115492554/adfa887f-a7ed-46f2-bd8e-afcba4599c04
QA has confirmed this is not repro in production https://expensify.slack.com/archives/C9YU7BX5M/p1707397831272999?thread_ts=1707394006.658679&cid=C9YU7BX5M
Curious what change from the recent deploy could have caused this
This is a regression from https://github.com/Expensify/App/pull/32471 where react-native-vision-camera
was upgraded from 2.16.2
to 2.16.5
. I tested with 2.16.7
and that version is still buggy. Since this is a deploy blocker we should go with the workaround suggested by @aswin-s photo={isCameraActive && props.photo}
.
π π π C+ reviewed Link to proposal
Current assignee @youssef-lr is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.
For internal people, discussing here https://expensify.slack.com/archives/C035J5C9FAP/p1707394239307519
I can take over, Youssef, if that is fine with you
Please assign me here as it seems related to hybrid PR I reviewed
Thank! I agree with @s77rt that given this is one of the last deploy blockers, the workaround is fine and we can add full fix upstream later.
π£ @situchan π An offer has been automatically sent to your Upwork account for the Contributor role π Thanks for contributing to the Expensify app!
Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review π§βπ» Keep in mind: Code of Conduct | Contributing π
π£ @aswin-s π An offer has been automatically sent to your Upwork account for the Contributor role π Thanks for contributing to the Expensify app!
Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review π§βπ» Keep in mind: Code of Conduct | Contributing π
@aswin-s can you raise the PR asap? thank you!
@mrousavy Since you are the maintainer of react-native-vision-camera
, can you please check why this is bugging out?
I think bug was introduced in https://github.com/mrousavy/react-native-vision-camera/pull/1955 which was included in 2.16.3
Hey - yup, we (@margelo) are on it! (cc @hannojg)
Issue is that the onDetachedWindow is called when it can already be attached again, so the commit you linked @mkhutornyi is the problem π
We'll revert that
Should we hold @aswin-s's PR then?
Yea, that PR is a workaround where the photo output is re-created each time the camera looses focus, which is a performance overhead as buffers need to be fully re-created. The true fix is reverting the PR 1955.
Thanks. @mountiny I think we should wait for @margelo's patch PR, pausing @aswin-s's one
@mrousavy @hannojg What is your ETA for a fix? Asking because the only other blocker has fix raised too and we would like to kick of the deploy as soon as possible.
I like doing a true fix but if that means postponing the deploy for 4+ hours maybe, I think the performance overhead of recreating the buffers should be fine