Skip to content
Merged
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 src/libs/API/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ function prepareRequest<TCommand extends ApiCommand, TKey extends OnyxKey>(
command,
data,
initiatedOffline: getIsOffline(),
requestID: requestIndex++,
requestIndex: requestIndex++,
...onyxDataWithoutOptimisticData,
successData,
failureData,
Expand Down
8 changes: 6 additions & 2 deletions src/libs/Errors/HttpsError.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
type HttpsErrorArguments = {message: string; status?: string; title?: string};
type HttpsErrorArguments = {message: string; status?: string; title?: string; requestID?: string};

/**
* Custom error class useful for re-throwing fetch errors with status code or valid error responses with status 200 but non 200 jsonCode
Expand All @@ -10,10 +10,14 @@ export default class HttpsError extends Error {

title: string;

constructor({message, status = '', title = ''}: HttpsErrorArguments) {
/** Server-issued requestID, when the failure was reported in a parsed response body. */
requestID?: string;

constructor({message, status = '', title = '', requestID}: HttpsErrorArguments) {
super(message);
this.name = 'HttpsError';
this.status = status;
this.title = title;
this.requestID = requestID;
}
}
2 changes: 2 additions & 0 deletions src/libs/HttpUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ function processHTTPRequest<TKey extends OnyxKey>(
message: CONST.ERROR.DUPLICATE_RECORD,
status: CONST.JSON_CODE.BAD_REQUEST.toString(),
title: CONST.ERROR_TITLE.DUPLICATE_RECORD,
requestID: response.requestID,
});
}

Expand All @@ -151,6 +152,7 @@ function processHTTPRequest<TKey extends OnyxKey>(
message: CONST.ERROR.EXPENSIFY_SERVICE_INTERRUPTED,
status: CONST.JSON_CODE.EXP_ERROR.toString(),
title: CONST.ERROR_TITLE.SOCKET,
requestID: response.requestID,
});
}

Expand Down
1 change: 1 addition & 0 deletions src/libs/Middleware/Logging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ const Logging: Middleware = (response, request) => {
message: error.message,
status: error.status,
title: error.title,
requestID: error.requestID,
request: sanitizeLogParams(request),
};

Expand Down
2 changes: 1 addition & 1 deletion src/libs/Middleware/SentryServerTiming.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const SentryServerTiming: Middleware = (response, request) => {
return response;
}

const spanId = `${group.spanOp}_${request.requestID}`;
const spanId = `${group.spanOp}_${request.requestIndex}`;
startSpan(spanId, {
name: group.spanName,
op: group.spanOp,
Expand Down
48 changes: 32 additions & 16 deletions src/libs/actions/PersistedRequests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ let persistedRequests: AnyRequest[] = [];
let ongoingRequest: AnyRequest | null = null;
let pendingSaveOperations: AnyRequest[] = [];
let isInitialized = false;
// Tracks all requestIDs this tab has ever seen (from disk init, save(), or other tabs).
// Tracks all request indexes this tab has ever seen (from disk init, save(), or other tabs).
// Used to distinguish stale own-write callbacks (ignore) from new requests enqueued
// by other browser tabs (merge into memory).
const knownRequestIDs = new Set<number>();
Expand All @@ -32,6 +32,10 @@ function trackOnyxWrite<T>(promise: Promise<T>): Promise<T> {
});
}

function getClientRequestIndex(request: {requestIndex?: number; requestID?: number}): number | undefined {
return request.requestIndex ?? request.requestID;
}

let initializationCallback: () => void;
function triggerInitializationCallback() {
if (typeof initializationCallback !== 'function') {
Expand Down Expand Up @@ -59,17 +63,21 @@ Onyx.connectWithoutView({
// correct in-memory state (Bug #80759 Issue 4).
// Exception 1: Onyx.clear() fires callback with null — allow through.
// Exception 2: Other browser tabs can enqueue requests. We detect these
// by checking for requestIDs not in knownRequestIDs, and merge them in.
// by checking for request indexes not in knownRequestIDs, and merge them in.
if (isInitialized && val != null) {
const newFromOtherTabs = val.filter((r) => r.requestID != null && !knownRequestIDs.has(r.requestID));
const newFromOtherTabs = val.filter((r) => {
const requestIndex = getClientRequestIndex(r);
return requestIndex != null && !knownRequestIDs.has(requestIndex);
});
if (newFromOtherTabs.length > 0) {
Log.info('[PersistedRequests] Merging requests from other tabs', false, {
newCount: newFromOtherTabs.length,
newCommands: getCommands(newFromOtherTabs),
});
for (const r of newFromOtherTabs) {
if (r.requestID != null) {
knownRequestIDs.add(r.requestID);
const requestIndex = getClientRequestIndex(r);
if (requestIndex != null) {
knownRequestIDs.add(requestIndex);
}
}
persistedRequests = [...persistedRequests, ...newFromOtherTabs];
Expand All @@ -86,12 +94,16 @@ Onyx.connectWithoutView({
if (pendingOnyxWrites === 0) {
const diskIDs = new Set<number>();
for (const r of val) {
if (r.requestID != null) {
diskIDs.add(r.requestID);
const requestIndex = getClientRequestIndex(r);
if (requestIndex != null) {
diskIDs.add(requestIndex);
}
}
const previousLength = persistedRequests.length;
persistedRequests = persistedRequests.filter((r) => r.requestID == null || diskIDs.has(r.requestID));
persistedRequests = persistedRequests.filter((r) => {
const requestIndex = getClientRequestIndex(r);
return requestIndex == null || diskIDs.has(requestIndex);
});
if (persistedRequests.length !== previousLength) {
Log.info('[PersistedRequests] Reconciled deletions from leader tab', false, {
removedCount: previousLength - persistedRequests.length,
Expand All @@ -106,10 +118,11 @@ Onyx.connectWithoutView({
const diskRequests = val ?? [];
persistedRequests = diskRequests;
for (const r of diskRequests) {
if (r.requestID == null) {
const requestIndex = getClientRequestIndex(r);
if (requestIndex == null) {
continue;
}
knownRequestIDs.add(r.requestID);
knownRequestIDs.add(requestIndex);
}

Log.info('[PersistedRequests] DISK vs MEMORY comparison', false, {
Expand All @@ -126,8 +139,9 @@ Onyx.connectWithoutView({
pendingCommands: getCommands(pendingSaveOperations),
});
for (const r of pendingSaveOperations) {
if (r.requestID != null) {
knownRequestIDs.add(r.requestID);
const requestIndex = getClientRequestIndex(r);
if (requestIndex != null) {
knownRequestIDs.add(requestIndex);
}
}
const requests = [...persistedRequests, ...pendingSaveOperations];
Expand Down Expand Up @@ -218,8 +232,9 @@ function save<TKey extends OnyxKey>(requestToPersist: Request<TKey>): Promise<vo
const requests = [...persistedRequests, requestToPersist];
const previousLength = persistedRequests.length;
persistedRequests = requests as AnyRequest[];
if (requestToPersist.requestID != null) {
knownRequestIDs.add(requestToPersist.requestID);
const requestIndex = getClientRequestIndex(requestToPersist as AnyRequest);
if (requestIndex != null) {
knownRequestIDs.add(requestIndex);
}

Log.info('[PersistedRequests] Request added to memory, persisting to disk', false, {
Expand Down Expand Up @@ -320,8 +335,9 @@ function update<TKey extends OnyxKey>(oldRequestIndex: number, newRequest: Reque
Log.info('[PersistedRequests] Updating a request', false, {oldRequest: sanitizeLogParams(oldRequest), newRequest: sanitizeLogParams(newRequest), oldRequestIndex});
requests.splice(oldRequestIndex, 1, newRequest as AnyRequest);
persistedRequests = requests;
if (newRequest.requestID != null) {
knownRequestIDs.add(newRequest.requestID);
const requestIndex = getClientRequestIndex(newRequest as AnyRequest);
if (requestIndex != null) {
knownRequestIDs.add(requestIndex);
}
return trackOnyxWrite(Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, requests));
}
Expand Down
4 changes: 2 additions & 2 deletions src/types/onyx/Request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ type RequestDataBase<TKey extends OnyxKey> = {
*/
initiatedOffline?: boolean;

/** The unique ID of the request */
requestID?: number;
/** The client-side monotonically-increasing index of this request (seeded with Date.now() at module load). Not to be confused with the server's response.requestID. */
requestIndex?: number;
};

/** Model of overall requests sent to the API */
Expand Down
4 changes: 2 additions & 2 deletions tests/actions/AppTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ describe('actions/App', () => {
command: 'AddComment',
successData: [{key: 'reportMetadata_1', onyxMethod: 'merge', value: {}}],
failureData: [{key: 'reportMetadata_2', onyxMethod: 'merge', value: {}}],
requestID: 123,
requestIndex: 123,
};

jest.spyOn(Navigation, 'clearPreloadedRoutes').mockImplementation(() => {});
Expand All @@ -134,7 +134,7 @@ describe('actions/App', () => {
expect.arrayContaining([
expect.objectContaining({
command: 'AddComment',
requestID: 123,
requestIndex: 123,
isRollback: true,
}),
]),
Expand Down
24 changes: 12 additions & 12 deletions tests/unit/PersistedRequests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const request: Request<'reportMetadata_1' | 'reportMetadata_2'> = {
command: 'OpenReport',
successData: [{key: 'reportMetadata_1', onyxMethod: 'merge', value: {}}],
failureData: [{key: 'reportMetadata_2', onyxMethod: 'merge', value: {}}],
requestID: 1,
requestIndex: 1,
};

beforeAll(() =>
Expand Down Expand Up @@ -55,7 +55,7 @@ describe('PersistedRequests', () => {
command: 'AddComment',
successData: [{key: 'reportMetadata_3', onyxMethod: 'merge', value: {}}],
failureData: [{key: 'reportMetadata_4', onyxMethod: 'merge', value: {}}],
requestID: 2,
requestIndex: 2,
};
PersistedRequests.save(request2);
PersistedRequests.processNextRequest();
Expand All @@ -70,7 +70,7 @@ describe('PersistedRequests', () => {
command: 'OpenReport',
successData: [{key: 'reportMetadata_1', onyxMethod: 'set', value: {}}],
failureData: [{key: 'reportMetadata_2', onyxMethod: 'set', value: {}}],
requestID: 3,
requestIndex: 3,
};
PersistedRequests.update(0, newRequest);
expect(PersistedRequests.getAll().at(0)).toEqual(newRequest);
Expand All @@ -81,7 +81,7 @@ describe('PersistedRequests', () => {
command: 'OpenReport',
successData: [{key: 'reportMetadata_1', onyxMethod: 'set', value: {}}],
failureData: [{key: 'reportMetadata_2', onyxMethod: 'set', value: {}}],
requestID: 4,
requestIndex: 4,
};
PersistedRequests.updateOngoingRequest(newRequest);
expect(PersistedRequests.getOngoingRequest()).toEqual(newRequest);
Expand All @@ -102,7 +102,7 @@ describe('PersistedRequests', () => {
command: 'OpenReport',
successData: [{key: 'reportMetadata_1', onyxMethod: 'set', value: {}}],
failureData: [{key: 'reportMetadata_2', onyxMethod: 'set', value: {}}],
requestID: 5,
requestIndex: 5,
data: {file: mockFile},
};

Expand Down Expand Up @@ -169,7 +169,7 @@ describe('PersistedRequests persistence guarantees', () => {
command: 'AddComment',
successData: [{key: 'reportMetadata_3', onyxMethod: 'merge', value: {}}],
failureData: [{key: 'reportMetadata_4', onyxMethod: 'merge', value: {}}],
requestID: 2,
requestIndex: 2,
};

PersistedRequests.save(requestB);
Expand Down Expand Up @@ -212,7 +212,7 @@ describe('PersistedRequests persistence guarantees', () => {
command: 'OpenReport',
successData: [{key: 'reportMetadata_1', onyxMethod: 'merge', value: {}}],
failureData: [{key: 'reportMetadata_2', onyxMethod: 'merge', value: {}}],
requestID: 30,
requestIndex: 30,
data: {file: mockFile},
};

Expand Down Expand Up @@ -263,14 +263,14 @@ describe('PersistedRequests persistence guarantees', () => {
command: 'CommandA',
successData: [{key: 'reportMetadata_1', onyxMethod: 'merge', value: {}}],
failureData: [{key: 'reportMetadata_2', onyxMethod: 'merge', value: {}}],
requestID: 10,
requestIndex: 10,
};

const requestB: Request<'reportMetadata_3' | 'reportMetadata_4'> = {
command: 'CommandB',
successData: [{key: 'reportMetadata_3', onyxMethod: 'merge', value: {}}],
failureData: [{key: 'reportMetadata_4', onyxMethod: 'merge', value: {}}],
requestID: 11,
requestIndex: 11,
};

// save(requestA): in-memory = [A], Onyx.set([A]) captured but not executed
Expand Down Expand Up @@ -314,13 +314,13 @@ describe('PersistedRequests persistence guarantees', () => {
command: 'CommandA',
successData: [{key: 'reportMetadata_1', onyxMethod: 'merge', value: {}}],
failureData: [{key: 'reportMetadata_2', onyxMethod: 'merge', value: {}}],
requestID: 20,
requestIndex: 20,
};
const requestB: Request<'reportMetadata_3' | 'reportMetadata_4'> = {
command: 'CommandB',
successData: [{key: 'reportMetadata_3', onyxMethod: 'merge', value: {}}],
failureData: [{key: 'reportMetadata_4', onyxMethod: 'merge', value: {}}],
requestID: 21,
requestIndex: 21,
};

PersistedRequests.save(requestA);
Expand Down Expand Up @@ -348,7 +348,7 @@ describe('PersistedRequests persistence guarantees', () => {
command: 'CommandC',
successData: [{key: 'reportMetadata_5', onyxMethod: 'merge', value: {}}],
failureData: [{key: 'reportMetadata_6', onyxMethod: 'merge', value: {}}],
requestID: 22,
requestIndex: 22,
};
PersistedRequests.save(requestC);
await waitForBatchedUpdates();
Expand Down
Loading