Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ios/ShareViewController/ShareViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class ShareViewController: UIViewController {
os_log("Saving file to: %@", filePath.path)

do {
try fileData.write(to: filePath, options: .completeFileProtection)
try fileData.write(to: filePath)
os_log("File saved successfully at: %@", filePath.path)
return filePath
} catch {
Expand Down
8 changes: 7 additions & 1 deletion src/libs/fileDownload/checkFileExists/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import fileURIToPath from '@libs/fileURIToPath';
import {logReceiptStatFailed} from '@libs/telemetry/ReceiptObservability';

import RNFS from 'react-native-fs';

Expand All @@ -22,7 +23,12 @@ function checkFileExists(path: string | undefined): Promise<boolean> {

const statIsFile = (candidate: string) => RNFS.stat(candidate).then((fileStat) => fileStat.isFile());

return statIsFile(decodedPath).catch(() => (decodedPath === rawPath ? false : statIsFile(rawPath).catch(() => false)));
const statFailed = (error: unknown) => {
logReceiptStatFailed(typeof error === 'object' && error !== null && 'code' in error ? String(error.code) : undefined);
return false;
};

return statIsFile(decodedPath).catch((error: unknown) => (decodedPath === rawPath ? statFailed(error) : statIsFile(rawPath).catch(statFailed)));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Retain the first stat error across the raw-path fallback

When a normal percent-encoded URI is checked while its actual decoded file is locked, the decoded RNFS.stat can fail with EPERM, but this branch discards that error and probes the literal %20/%23 path next; that nonexistent fallback then reports ENOENT, which is the only code sent to receipt telemetry. This makes locked files look deleted in precisely the encoded-path scenario the new telemetry is intended to diagnose, so preserve the decoded-path error or report both failures when the fallback also fails.

Useful? React with 👍 / 👎.

}

export default checkFileExists;
8 changes: 8 additions & 0 deletions src/libs/telemetry/ReceiptObservability.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,13 @@ function logReceiptDropped({
});
}

function logReceiptStatFailed(code: string | undefined) {
Log.info(`${RECEIPT_LOG_PREFIX} stat failed`, false, {
event: 'statFailed',
code,
});
}

function logReceiptAdoptFailed({error, captureSource}: {error: unknown; captureSource: ReceiptCaptureSource}) {
Log.alert(`${RECEIPT_LOG_PREFIX} adopt failed`, {
event: 'adoptFailed',
Expand Down Expand Up @@ -246,6 +253,7 @@ export {
logReceiptSubmitted,
logReceiptEnqueued,
logReceiptDropped,
logReceiptStatFailed,
logReceiptAdoptFailed,
logReceiptQueueSnapshot,
getPickerCaptureSource,
Expand Down
2 changes: 1 addition & 1 deletion src/libs/telemetry/forwardLogsToSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ type ForwardedLogPrefix = TupleToUnion<typeof FORWARDED_LOG_PREFIXES>;
* receipt keys tied to the receipt logs instead of widening the global whitelist.
*/
const PREFIX_SCOPED_PARAMETERS_WHITELIST = new Map<ForwardedLogPrefix, ReadonlyArray<string | RegExp>>([
['[Receipt]', ['receiptTraceId', 'transactionID', 'event', 'captureSource']],
['[Receipt]', ['receiptTraceId', 'transactionID', 'event', 'captureSource', 'code']],
['[PDFStall]', ['reportID']],
['[OpenReportStall]', ['reportID']],
]);
Expand Down
39 changes: 27 additions & 12 deletions src/pages/Share/SubmitDetailsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,11 @@ import {rand64} from '@libs/NumberUtils';
import {isTrackOnboardingChoice} from '@libs/OnboardingUtils';
import {getParticipantsOption, getReportOption} from '@libs/OptionsListUtils';
import {hasOnlyPersonalPolicies as hasOnlyPersonalPoliciesUtil, isGroupPolicy} from '@libs/PolicyUtils';
import ReceiptStorage from '@libs/ReceiptStorage';
import {shouldValidateFile} from '@libs/ReceiptUtils';
import {getReportOrDraftReport, isMoneyRequestReport, isSelfDM} from '@libs/ReportUtils';
import {cancelSpan, endSpan} from '@libs/telemetry/activeSpans';
import {logReceiptCaptured, logReceiptSubmitted, mintAndStampReceiptTraceId} from '@libs/telemetry/ReceiptObservability';
import {logReceiptAdoptFailed, logReceiptCaptured, logReceiptSubmitted, mintAndStampReceiptTraceId} from '@libs/telemetry/ReceiptObservability';
import {cancelTracking} from '@libs/telemetry/submitFollowUpAction';
import {getDefaultTaxCode, getIsFromGlobalCreate, getTaxValue} from '@libs/TransactionUtils';

Expand Down Expand Up @@ -563,17 +564,31 @@ function SubmitDetailsPage({
return;
}
formHasBeenSubmitted.current = true;
readFileAsync(
currentReceiptSource,
currentReceiptName,
(file) => onSuccess(file, locationPermissionGranted),
() => {
// Allow retry after a file-read failure.
formHasBeenSubmitted.current = false;
setIsConfirming(false);
},
currentReceiptType,
);
// The share extension wipes its folder on the next share. Adopting at submit, not at ingestion, keeps cancelled
// shares out of a folder nothing prunes.
ReceiptStorage.adopt(currentReceiptSource, currentReceiptName)
.then((durableName) => {
const uri = ReceiptStorage.toLocalUri(durableName);
// The shared path is empty once the move lands, and the draft is what a retry re-reads.
setMoneyRequestReceipt(CONST.IOU.OPTIMISTIC_TRANSACTION_ID, uri, currentReceiptName, true, currentReceiptType);
return uri;
})
.catch((error: unknown) => {
logReceiptAdoptFailed({error, captureSource: 'share'});
return currentReceiptSource;
})
.then((uri) =>
readFileAsync(
uri,
currentReceiptName,
(file) => onSuccess(file, locationPermissionGranted),
() => {
formHasBeenSubmitted.current = false;
setIsConfirming(false);
},
currentReceiptType,
),
);
};

const onConfirm = (gpsRequired?: boolean) => {
Expand Down
66 changes: 66 additions & 0 deletions tests/ui/SubmitDetailsPageTest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {readFileAsync} from '@libs/fileDownload/FileUtils';
import getIsNarrowLayout from '@libs/getIsNarrowLayout';
import Log from '@libs/Log';
import Navigation from '@libs/Navigation/Navigation';
import ReceiptStorage from '@libs/ReceiptStorage';
import {getReportOrDraftReport} from '@libs/ReportUtils';
import {Scheduler} from '@libs/Scheduler';

Expand Down Expand Up @@ -64,6 +65,16 @@ jest.mock('@libs/fileDownload/FileUtils', () => {
};
});

// Jest resolves the bare specifier to the web no-op, so mock the shape and let each test opt into the native move.
jest.mock('@libs/ReceiptStorage', () => ({
__esModule: true,
default: {
adopt: jest.fn((uriOrPath: string) => Promise.resolve(uriOrPath)),
toLocalUri: jest.fn((durableName: string) => durableName),
resolve: jest.fn((source: string) => source),
},
}));

jest.mock('@libs/getCurrentPosition', () => jest.fn());

jest.mock('@hooks/useNetwork', () => jest.fn(() => ({isOffline: false})));
Expand Down Expand Up @@ -344,6 +355,61 @@ describe('SubmitDetailsPage', () => {
expect(jest.mocked(readFileAsync)).not.toHaveBeenCalled();
});

// Error #12 — the share extension wipes its folder on the next share, so the read must come from the receipts folder.
it('adopts the shared file into the receipts folder and uploads from there', async () => {
// Given a share whose file adopt moves into the receipts folder
const durableUri = 'file:///Documents/Receipts-Upload/shared_1234.jpg';
jest.mocked(ReceiptStorage.adopt).mockResolvedValueOnce('shared_1234.jpg');
jest.mocked(ReceiptStorage.toLocalUri).mockReturnValueOnce(durableUri);

// When the user confirms the submit
await renderAndConfirm();

// Then the upload reads the durable copy, never the app-group path
expect(ReceiptStorage.adopt).toHaveBeenCalledWith('file://shared.jpg', expect.any(String));
expect(jest.mocked(readFileAsync).mock.calls.at(0)?.[0]).toBe(durableUri);
});

// Error #12b — a failed move must not block the submit: the shared path still works for this session.
it('submits with the shared path when adopting the file fails', async () => {
// Given adopt rejects
const logAlertSpy = jest.spyOn(Log, 'alert').mockImplementation(() => {});
jest.mocked(ReceiptStorage.adopt).mockRejectedValueOnce(new Error('move failed'));

// When the user confirms the submit
await renderAndConfirm();

// Then the upload falls back to the shared path, logs the failure, and the expense is still created
expect(jest.mocked(readFileAsync).mock.calls.at(0)?.[0]).toBe('file://shared.jpg');
expect(logAlertSpy).toHaveBeenCalledWith(expect.stringContaining('adopt failed'), expect.objectContaining({captureSource: 'share'}));
expect(TrackExpense.requestMoney).toHaveBeenCalled();

logAlertSpy.mockRestore();
});

// Error #12c — adopt is a move, so a retry after a read failure has to work off the durable copy.
it('retries from the durable copy after a file-read failure', async () => {
// Given a share that adopts fine but fails its first read
const durableUri = 'file:///Documents/Receipts-Upload/shared_1234.jpg';
jest.mocked(ReceiptStorage.adopt).mockResolvedValueOnce('shared_1234.jpg');
jest.mocked(ReceiptStorage.toLocalUri).mockReturnValueOnce(durableUri);
jest.mocked(readFileAsync).mockImplementationOnce((_path, _fileName, _onSuccess, onFailure) => {
onFailure?.('[FileUtils] Could not read uploaded file');
return Promise.resolve();
});

// When the user confirms, the read fails, and the user confirms again
await renderAndConfirm();
fireEvent.press(screen.getByTestId('mock-confirm-button'));
await waitForBatchedUpdatesWithAct();

// Then the second attempt works off the durable copy, not the shared path the move emptied
expect(jest.mocked(readFileAsync).mock.calls.at(0)?.[0]).toBe(durableUri);
expect(jest.mocked(ReceiptStorage.adopt).mock.calls.at(1)?.[0]).toBe(durableUri);
expect(jest.mocked(readFileAsync).mock.calls.at(1)?.[0]).toBe(durableUri);
expect(TrackExpense.requestMoney).toHaveBeenCalled();
});

// Error #5 — wide layout fallback: when destination is not topmost, reveal it via revealRouteBeforeDismissingModal
// and defer navigation to cleanup (shouldNavigate: false) so we do not double-navigate after dismiss.
it('wide layout: reveals destination via revealRouteBeforeDismissingModal when another report is topmost', async () => {
Expand Down
16 changes: 16 additions & 0 deletions tests/unit/checkFileExistsTest.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import checkFileExists from '@libs/fileDownload/checkFileExists/index';
import Log from '@libs/Log';

import type RNFS from 'react-native-fs';

Expand Down Expand Up @@ -99,6 +100,21 @@ describe('checkFileExists', () => {
expect(result).toBe(false);
});

it('should log the stat error code so a locked device is distinguishable from a missing file', async () => {
// Given a stat that fails the way a locked device fails it
const logInfoSpy = jest.spyOn(Log, 'info').mockImplementation(() => {});
mockStat.mockRejectedValue(Object.assign(new Error('Operation not permitted'), {code: 'EPERM'}));

// When the file is checked
const result = await checkFileExists('/var/mobile/Containers/sharedFiles/locked.jpg');

// Then it still reports missing, but the code reaches telemetry
expect(result).toBe(false);
expect(logInfoSpy).toHaveBeenCalledWith(expect.stringContaining('stat failed'), false, {event: 'statFailed', code: 'EPERM'});

logInfoSpy.mockRestore();
});

it('should return false when path is a directory', async () => {
mockStat.mockResolvedValue(buildStatResult(false));
const result = await checkFileExists('/var/mobile/Containers/');
Expand Down
Loading