Skip to content

Commit

Permalink
Revert "feat: fallback to NoopProvider if we run into OOM [2/3]"
Browse files Browse the repository at this point in the history
  • Loading branch information
tgolen authored Mar 5, 2024
1 parent 69b2d59 commit 651540b
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 208 deletions.
1 change: 0 additions & 1 deletion lib/storage/__mocks__/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ const set = jest.fn((key, value) => {
});

const idbKeyvalMock: StorageProvider = {
name: 'KeyValMockProvider',
init: () => undefined,
setItem(key, value) {
return set(key, value);
Expand Down
136 changes: 45 additions & 91 deletions lib/storage/index.ts
Original file line number Diff line number Diff line change
@@ -1,52 +1,13 @@
import * as Logger from '../Logger';

import PlatformStorage from './platforms';
import InstanceSync from './InstanceSync';
import NoopProvider from './providers/NoopProvider';
import type StorageProvider from './providers/types';

let provider = PlatformStorage;
const provider = PlatformStorage;
let shouldKeepInstancesSync = false;
let finishInitalization: (value?: unknown) => void;
const initPromise = new Promise((resolve) => {
finishInitalization = resolve;
});

type Storage = {
getStorageProvider: () => StorageProvider;
} & Omit<StorageProvider, 'name'>;

/**
* Degrade performance by removing the storage provider and only using cache
*/
function degradePerformance(error: Error) {
Logger.logAlert(`Error while using ${provider.name}. Falling back to only using cache and dropping storage.`);
console.error(error);
provider = NoopProvider;
}

/**
* Runs a piece of code and degrades performance if certain errors are thrown
*/
function tryOrDegradePerformance<T>(fn: () => Promise<T> | T): Promise<T> {
return new Promise<T>((resolve, reject) => {
initPromise.then(() => {
try {
resolve(fn());
} catch (error) {
// Test for known critical errors that the storage provider throws, e.g. when storage is full
if (error instanceof Error) {
// IndexedDB error when storage is full (https://github.com/Expensify/App/issues/29403)
if (error.message.includes('Internal error opening backing store for indexedDB.open')) {
degradePerformance(error);
}
}

reject(error);
}
});
});
}
} & StorageProvider;

const Storage: Storage = {
/**
Expand All @@ -61,119 +22,112 @@ const Storage: Storage = {
* and enables fallback providers if necessary
*/
init() {
tryOrDegradePerformance(provider.init).finally(() => {
finishInitalization();
});
provider.init();
},

/**
* Get the value of a given key or return `null` if it's not available
*/
getItem: (key) => tryOrDegradePerformance(() => provider.getItem(key)),
getItem: (key) => provider.getItem(key),

/**
* Get multiple key-value pairs for the give array of keys in a batch
*/
multiGet: (keys) => tryOrDegradePerformance(() => provider.multiGet(keys)),
multiGet: (keys) => provider.multiGet(keys),

/**
* Sets the value for a given key. The only requirement is that the value should be serializable to JSON string
*/
setItem: (key, value) =>
tryOrDegradePerformance(() => {
const promise = provider.setItem(key, value);
setItem: (key, value) => {
const promise = provider.setItem(key, value);

if (shouldKeepInstancesSync) {
return promise.then(() => InstanceSync.setItem(key));
}
if (shouldKeepInstancesSync) {
return promise.then(() => InstanceSync.setItem(key));
}

return promise;
}),
return promise;
},

/**
* Stores multiple key-value pairs in a batch
*/
multiSet: (pairs) => tryOrDegradePerformance(() => provider.multiSet(pairs)),
multiSet: (pairs) => provider.multiSet(pairs),

/**
* Merging an existing value with a new one
*/
mergeItem: (key, changes, modifiedData) =>
tryOrDegradePerformance(() => {
const promise = provider.mergeItem(key, changes, modifiedData);
mergeItem: (key, changes, modifiedData) => {
const promise = provider.mergeItem(key, changes, modifiedData);

if (shouldKeepInstancesSync) {
return promise.then(() => InstanceSync.mergeItem(key));
}
if (shouldKeepInstancesSync) {
return promise.then(() => InstanceSync.mergeItem(key));
}

return promise;
}),
return promise;
},

/**
* Multiple merging of existing and new values in a batch
* This function also removes all nested null values from an object.
*/
multiMerge: (pairs) => tryOrDegradePerformance(() => provider.multiMerge(pairs)),
multiMerge: (pairs) => provider.multiMerge(pairs),

/**
* Removes given key and its value
*/
removeItem: (key) =>
tryOrDegradePerformance(() => {
const promise = provider.removeItem(key);
removeItem: (key) => {
const promise = provider.removeItem(key);

if (shouldKeepInstancesSync) {
return promise.then(() => InstanceSync.removeItem(key));
}
if (shouldKeepInstancesSync) {
return promise.then(() => InstanceSync.removeItem(key));
}

return promise;
}),
return promise;
},

/**
* Remove given keys and their values
*/
removeItems: (keys) =>
tryOrDegradePerformance(() => {
const promise = provider.removeItems(keys);
removeItems: (keys) => {
const promise = provider.removeItems(keys);

if (shouldKeepInstancesSync) {
return promise.then(() => InstanceSync.removeItems(keys));
}
if (shouldKeepInstancesSync) {
return promise.then(() => InstanceSync.removeItems(keys));
}

return promise;
}),
return promise;
},

/**
* Clears everything
*/
clear: () =>
tryOrDegradePerformance(() => {
if (shouldKeepInstancesSync) {
return InstanceSync.clear(() => provider.clear());
}
clear: () => {
if (shouldKeepInstancesSync) {
return InstanceSync.clear(() => provider.clear());
}

return provider.clear();
}),
return provider.clear();
},

// This is a noop for now in order to keep clients from crashing see https://github.com/Expensify/Expensify/issues/312438
setMemoryOnlyKeys: () => tryOrDegradePerformance(() => provider.setMemoryOnlyKeys()),
setMemoryOnlyKeys: () => provider.setMemoryOnlyKeys(),

/**
* Returns all available keys
*/
getAllKeys: () => tryOrDegradePerformance(() => provider.getAllKeys()),
getAllKeys: () => provider.getAllKeys(),

/**
* Gets the total bytes of the store
*/
getDatabaseSize: () => tryOrDegradePerformance(() => provider.getDatabaseSize()),
getDatabaseSize: () => provider.getDatabaseSize(),

/**
* @param onStorageKeyChanged - Storage synchronization mechanism keeping all opened tabs in sync (web only)
*/
keepInstancesSync(onStorageKeyChanged) {
// If InstanceSync is null, it means we're on a native platform and we don't need to keep instances in sync
if (InstanceSync === null) return;
if (InstanceSync == null) return;

shouldKeepInstancesSync = true;
InstanceSync.init(onStorageKeyChanged);
Expand Down
4 changes: 0 additions & 4 deletions lib/storage/providers/IDBKeyValProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ import type {Value} from './types';
let idbKeyValStore: UseStore;

const provider: StorageProvider = {
/**
* The name of the provider that can be printed to the logs
*/
name: 'IDBKeyValProvider',
/**
* Initializes the storage provider
*/
Expand Down
103 changes: 0 additions & 103 deletions lib/storage/providers/NoopProvider.ts

This file was deleted.

4 changes: 0 additions & 4 deletions lib/storage/providers/SQLiteProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,6 @@ const DB_NAME = 'OnyxDB';
let db: QuickSQLiteConnection;

const provider: StorageProvider = {
/**
* The name of the provider that can be printed to the logs
*/
name: 'SQLiteProvider',
/**
* Initializes the storage provider
*/
Expand Down
6 changes: 1 addition & 5 deletions lib/storage/providers/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ type KeyValuePairList = KeyValuePair[];
type OnStorageKeyChanged = (key: Key, value: Value | null) => void;

type StorageProvider = {
/**
* The name of the provider that can be printed to the logs
*/
name: string;
/**
* Initializes the storage provider
*/
Expand Down Expand Up @@ -86,4 +82,4 @@ type StorageProvider = {
};

export default StorageProvider;
export type {Value, Key, KeyList, KeyValuePair, KeyValuePairList, OnStorageKeyChanged};
export type {Value, Key, KeyList, KeyValuePairList, OnStorageKeyChanged};

0 comments on commit 651540b

Please sign in to comment.