[HOLD for payment 2024-12-30] BUG: Receipt is deleted if app is killed while request is in queue

[neil-marcellini]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: v9.0.55-9
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
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: See the linked issue below
Expensify/Expensify Issue URL: https://github.com/Expensify/Expensify/issues/434533
Issue reported by: @neil Marcellini

Originally reported internally here

Slack conversation https://expensify.slack.com/archives/C049HHMV9SM/p1730315753190039

Action Performed:

1. Go offline
2. Submit an expense and scan a receipt with the in app camera, submit it to anyone
3. Kill the app
4. Re-open the app
5. Go back online

Expected Result:

The receipt is uploaded and scanned correctly. If it fails for some reason, then the user is able to download the receipt.

Desired Result:

Store images captured with the in app camera to a non-temporary folder in the application folder within the file system. For example, store images in a folder Receipts-Pending-Upload under the New Expensify folder.

We're still discussing in the thread linked below, but maybe after a receipt has been uploaded on the server, delete it from this folder.

Actual Result:

There is an error submitting the expense and no option to download the receipt. If the user dismisses this error then they permanently lose the receipt image.

Notes

I think this happens because we store the receipts in a /tmp/ folder which is cleared when the app is killed (E.g. file:///private/var/mobile/Containers/Data/Application/C5B31E5E-C6BD-4372-B1DF-D1326BDA7B28/tmp/9C8642AF-C0FF-44F9-988A-10B3A752B7A2.jpeg). It's a common nervous habit of iOS users to kill apps whenever they go to the home screen, so it's pretty realistic that this can happen, especially if the SequentialQueue gets a bit backed up.

More discussion in #quality.

Looks like if we upgrade react-native-vision-camera we can specify a path when taking the photo. Or we could manually move the file to our desired location, if upgrading vision camera is really difficult.

Workaround:

Try to take screen shots of the receipt images before dismissing the error and try re-uploading later. Not a very good workaround especially when users trust our app not to lose data.

Platforms:

Which of our officially supported platforms is this issue occurring on?

iOS Standalone, maybe others.

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

OfflineReceiptLoss_10-30-2024.11-48-37_1.MP4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @johncschuster

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @allgandalf (External)

[melvin-bot]

Triggered auto assignment to @johncschuster (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[ikevin127]

From taking a quick look at what happens to the RequestMoney call which was stored in the sequential queue and resumed once back online, I'm not entirely sure that the receipt is deleted from the API call, it looks like the receipt [Object] is still there and passed with the call upon returning back online when the call being made.

What I noticed instead is a 405 response from the BE regarding the RequestMoney call, mentioning that no amount was specified, which I think is what leads to the red error which if dismissed -> removes the expense entirely, giving the sense that the receipt was lose because of closing the app, when in fact the expense call failed and when dismissed, the expense goes away completely.

Note: I also noticed a Onyx merge error once the call is being made when returning back online, most likely coming from the BE response in onyx data.

I'd look into this from a BE perspective to see what happens there, if the receipt is indeed passed and why does the BE respond with error in this issue's case. I'm assuming it might have something to do with the scan being intrerupted by the user closing the app while offline, then getting back online and call being resumed from squential queue, since the BE response is error: no amount was passed.

[neil-marcellini]

Hi, thanks for taking a look @ikevin127. There's a lot more context that I forgot to add to the issue description. I'll start adding that now. I'll also work on doing some backend testing to verify that the receipt file is not received in this case. I'm having a bit of trouble getting the iOS simulator to connect to my dev environment, which is why I haven't been able to gather that info yet. However, I'm pretty sure that's what's happening.

[neil-marcellini]

The description is updated now.

[ikevin127]

I deployed iOS: Native on a real device and was able to do some debugging, looks like this is a matter of the temporary file having a limited lifetime and being deleted if the app is closed for x amount of time (minute or so).

[neil-marcellini]

What were your conclusions from debugging? I see you crossed part out.

[ikevin127]

@neil-marcellini It's confusing but here's what I found:

The root cause does not seem related to the device saving the file temporarely but rather to this line -> where we save the source here:

App/src/libs/actions/IOU.ts

Lines 458 to 463 in bef062b

	function setMoneyRequestReceipt(transactionID: string, source: string, filename: string, isDraft: boolean, type?: string) {
	Onyx.merge(`${isDraft ? ONYXKEYS.COLLECTION.TRANSACTION_DRAFT : ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, {
	receipt: {source, type: type ?? ''},
	filename,
	});
	}

because in our case isDraft is always true meaning we always set the receipt source to ONYXKEYS.COLLECTION.TRANSACTION_DRAFT instead of ONYXKEYS.COLLECTION.TRANSACTION.

Then we have this part:

App/src/libs/actions/IOU.ts

Lines 840 to 849 in bef062b

	{
	onyxMethod: Onyx.METHOD.MERGE,
	key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`,
	value: {
	// Disabling this line since transaction.filename can be an empty string
	// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
	errors: getReceiptError(transaction.receipt, transaction.filename || transaction.receipt?.filename, isScanRequest, errorKey),
	pendingFields: clearedPendingFields,
	},
	},

where we build the RequestMoney API call onyx failureData we're passing the transaction.receipt via IOU.setMoneyRequestReceipt.

What I noticed about setMoneyRequestReceipt is that if we change the code like this:

function setMoneyRequestReceipt(transactionID: string, source: string, filename: string, isDraft: boolean, type?: string) {
    if (isDraft) {
        Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {
            receipt: {source, type: type ?? ''},
            filename,
        });
    }   

    Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, {
        receipt: {source, type: type ?? ''},
        filename,
    });
}

Then when returning back online and transaction fails, we always get the Download receipt button which allows us to download the receipt - regardless of how long the app was killed for.

ScreenRecording_11-04-2024.14-40-44_1.MP4

This is the confusing part as I was not able to figure out what happens BE wise when returning back online and why changing setMoneyRequestReceipt as mentioned above always returns the Download receipt error whereas with the current code that never happens on staging even if closing / reopening the app right away, but it can happen on dev sometimes.

[allgandalf]

@neil-marcellini thoughts on ^

[neil-marcellini]

Interesting, thanks for posting your findings. I managed to test this on dev with an iOS simulator. Of course I can't take a photo but choosing an image from the photo library had the same result. I had to leave the app killed for about a minute to reproduce the issue, which isn't shown in the video to save time.

I confirmed by debugging the backend that the receiptFile is empty. I will try your suggested solution and see if that fixes it.

ScanDeleted2024-11-06.at.12.47.20.mp4

[neil-marcellini]

The root cause does not seem related to the device saving the file temporarely but rather to this line -> where we save the source here:

I'm not quite sure I'm following. When requestMoney is called on the confirmation page the receiptFile is passed.

App/src/pages/iou/request/step/IOURequestStepConfirmation.tsx

Lines 500 to 503 in ab40264

	requestMoney(selectedParticipants, trimmedComment, receiptFile, {
	lat: successData.coords.latitude,
	long: successData.coords.longitude,
	});

If you trace that down to where the failure data is generated I'm pretty sure it all works.

[neil-marcellini]

I added a suggestion about how we might store the photo in a non temp location.

[rezkiy37]

Hi, I am Michael (Mykhailo) from Callstack, an expert agency and I can work on this issue.

[neil-marcellini]

Thanks so much. Please post a proposal for how you're planning to fix this.

[allgandalf]

@rezkiy37 when can we expect a proposal for the fix?

[rezkiy37]

I am working on the proposal. I will post it next week.

[neil-marcellini]

Heads up, I'll OOO next week. If you want a faster review you can ask @johncschuster to get another internal reviewer.

[allgandalf]

friendly bump for the copy @NickTooker

[NickTooker]

The receipt didn't upload, but it's safely [stored on your device here](link to file on device). Please try again later.

I think Neil's copy and reasoning are sound. Let's keep the same first sentence (didn't vs failed) which makes the entire message more casual/approachable IMO. Let's also re-add the CTA at the end to try again.

[melvin-bot]

@johncschuster, @neil-marcellini, @NickTooker, @rezkiy37, @allgandalf Whoops! This issue is 2 days overdue. Let's get this updated quick!

[rezkiy37]

No overdue. The PR has been approved and is waiting for merging. Also, I will work on the second PR to fix the receipt uploading (#52483 (comment)).

[allgandalf]

♻️ Update: PR was merged

[rezkiy37]

Working on the blob issue.

[rezkiy37]

It seems like I've found a solution. Basically, I am going to recreate a blob before the request. There is a function

App/src/libs/fileDownload/FileUtils.ts

Lines 168 to 174 in 2e8f4fa

	/**
	* Reads a locally uploaded file
	* @param path - the blob url of the locally uploaded file
	* @param fileName - name of the file to read
	*/
	const readFileAsync: ReadFileAsync = (path, fileName, onSuccess, onFailure = () => {}, fileType = '') =>
	new Promise((resolve) => {

that the app utilizes for creating blobs from files. It works properly in our case when the user scans a receipt offline, kills the app, and then opens and goes online.

Video

IOS.mp4

Screenshot and receipt

URL - https://www.expensify.com/receipts/w_4812412a80340a1e10eb45117c112ae80510bc1e.jpg

Working on a PR.

[allgandalf]

Thanks for the update @rezkiy37

[rezkiy37]

Working on the PR - #54358.

[rezkiy37]

There are some troubles with building IOS and Android currently. So I am coding but hopefully, I will test it on Monday.

[allgandalf]

yeah builds were failing, but if you merge main now, all builds should be successful!

[rezkiy37]

I've been able to launch IOS today. I am polishing the PR today. Also, I still want to figure out why the blob breaks down after the app refresh.

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.77-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Store receipts in stable directory on the native platforms #52483

If no regressions arise, payment will be issued on 2024-12-30. 🎊

For reference, here are some details about the assignees on this issue:

• @rezkiy37 does not require payment (Contractor)
• @allgandalf requires payment (Needs manual offer from BZ)
• @neil requires payment (Needs manual offer from BZ)

[melvin-bot]

@allgandalf @johncschuster @allgandalf The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[allgandalf]

@johncschuster please remove the hold for payment label here, we are working on PR part 2 of this issue, thanks :)

[rezkiy37]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Submitting the scanned expense is errored when the user kills the app while the request is in the queue.

What is the root cause of that problem?

The app utilizes Blob to store and send receipts. The issue arises because files, such as File instances, are not serializable into JSON to store in Onyx. Then, the app parses this stringified version back, the File instance cannot be reconstructed in its original form.

What changes do you think we should make in order to solve the problem?

I propose reconstructing a blob of the receipt if the request is created while the user is offline. I am going to intercept the blob right there:

App/src/libs/HttpUtils.ts

Lines 162 to 167 in 52b4eca

	Object.keys(data).forEach((key) => {
	if (typeof data[key] === 'undefined') {
	return;
	}
	formData.append(key, data[key] as string | Blob);
	});

I require an additional property (initiatedOffline) to be added to a request to indicate whether it was created offline to

App/src/types/onyx/Request.ts

Line 23 in 52b4eca

type RequestData = {

The draft implementation is

if (key === 'receipt' && initiatedOffline) {
    const {uri: path = '', source} = data[key] as File;

    return import('./fileDownload/FileUtils')
        .then(({readFileAsync}) => readFileAsync(source, path, () => {}))
        .then((file) => {
            if (!file) {
                return;
            }

            formData.append(key, file);
        })
        .catch(() => {
            console.debug('[dev] Error reading photo');
        });
}

I use the dynamic import because there is a circle-import issue (Slack).

Draft PR - https://github.com/Expensify/App/pull/54358/files.

What alternative solutions did you explore? (Optional)

1. Nothing. Since we've integrated the stable receipt directory the user can recreate an expense.

[allgandalf]

I will look in detail at the proposal tomorrow, just thinking out loud, the new reconstructed blob will still be attached to the same reportID right ? so in this case we just replace the previous image? i also wondered that how were we able to see the thumbnail for previous cases ?

[rezkiy37]

will still be attached to the same reportID right ?

Yes, right.

so in this case we just replace the previous image?

Not actually. We use the same image (path). We recreating the blob object to send the correct one.

[allgandalf]

We recreating the blob object to send the correct one.

Ohh i re-read what you wrote, got it! thanks!!!, makes sense to me mostly but still i will review and let you know tomorrow

and Merry Christmas 😄

[rezkiy37]

We recreating the blob object to send the correct one.

Ohh i re-read what you wrote, got it! thanks!!!, makes sense to me mostly but still i will review and let you know tomorrow

and Merry Christmas 😄

Merry Christmas 🎄🎁