From 968ec3b430dbce25e838604467677a9567ec0983 Mon Sep 17 00:00:00 2001 From: pikiou Date: Tue, 22 Feb 2022 02:10:16 +0100 Subject: [PATCH] Fix gsheet api request saturation caused by an infinite delayed loop --- src/server/gsheets/accessors.ts | 51 +++++++++++++++----------- src/server/gsheets/expressAccessors.ts | 38 +++++++++++++++---- src/server/notifications.ts | 4 +- 3 files changed, 61 insertions(+), 32 deletions(-) diff --git a/src/server/gsheets/accessors.ts b/src/server/gsheets/accessors.ts index 5327ccd..2a03f4b 100644 --- a/src/server/gsheets/accessors.ts +++ b/src/server/gsheets/accessors.ts @@ -11,8 +11,9 @@ export { SheetNames } from "./localDb" const CRED_PATH = path.resolve(process.cwd(), "access/gsheets.json") -const REMOTE_UPDATE_DELAY = 80000 +const REMOTE_UPDATE_DELAY = 40000 const DELAY_BETWEEN_ATTEMPTS = 10000 +const DELAY_BETWEEN_FIRST_LOAD = 1500 let creds: string | undefined | null @@ -44,7 +45,7 @@ export async function checkGSheetsAccess(): Promise { console.error(`Google Sheets: no creds found, loading local database instead`) } } -export function getSheet< +export async function getSheet< // eslint-disable-next-line @typescript-eslint/ban-types ElementNoId extends object, Element extends ElementNoId & ElementWithId @@ -52,18 +53,17 @@ export function getSheet< sheetName: keyof SheetNames, specimen: Element, translation: { [k in keyof Element]: string } -): Sheet { +): Promise> { + let sheet: Sheet if (!sheetList[sheetName]) { - sheetList[sheetName] = new Sheet(sheetName, specimen, translation) + sheet = new Sheet(sheetName, specimen, translation) + await sheet.waitForFirstLoad() + sheetList[sheetName] = sheet + setInterval(() => sheet.dbUpdate(), REMOTE_UPDATE_DELAY) + } else { + sheet = sheetList[sheetName] as Sheet } - const sheet = sheetList[sheetName] as Sheet - - setTimeout( - () => setInterval(() => sheet.dbUpdate(), REMOTE_UPDATE_DELAY), - 1000 * Object.values(sheetList).length - ) - return sheet } @@ -100,8 +100,6 @@ export class Sheet< _.invert(translation), (englishProp: string) => (specimen as any)[englishProp] ) as Element - - setTimeout(() => this.dbFirstLoad(), 100 * Object.values(sheetList).length) } async getList(): Promise { @@ -154,6 +152,15 @@ export class Sheet< } } + async waitForFirstLoad(): Promise { + setTimeout( + () => this.dbFirstLoad(), + DELAY_BETWEEN_FIRST_LOAD * Object.values(sheetList).length + ) + + await this.waitForLoad() + } + private async waitForLoad(): Promise { return new Promise((resolve, _reject) => { this.addToRunAfterLoad(() => resolve(undefined)) @@ -194,10 +201,9 @@ export class Sheet< async dbLoad(): Promise { try { if (await hasGSheetsAccess()) { - this.dbLoadAsync().then(() => this.doRunAfterLoad()) - } else { - this.doRunAfterLoad() + await this.dbLoadAsync() } + this.doRunAfterLoad() } catch (e) { console.error("Error in dbLoad: ", e) } @@ -214,6 +220,7 @@ export class Sheet< if (!(await hasGSheetsAccess())) { await this.loadLocalDb() } else if (this.toRunAfterLoad && __DEV__) { + // Save once this.toRunAfterLoad.push(() => this.saveLocalDb()) } @@ -288,7 +295,7 @@ export class Sheet< private async dbLoadAsync(): Promise { type StringifiedElement = Record - const sheet = await this.getGSheet() + const sheet = await this.getGSheet(20) if (!sheet) { return @@ -326,7 +333,7 @@ export class Sheet< }) } - private async getGSheet(): Promise { + private async getGSheet(attempts = 3): Promise { return tryNTimes( async () => { if (creds === undefined) { @@ -347,7 +354,7 @@ export class Sheet< return doc.sheetsByTitle[this.sheetName] }, () => null, - 20, + attempts, DELAY_BETWEEN_ATTEMPTS / 5 ) } @@ -590,7 +597,7 @@ function parseDate(value: string): Date { async function tryNTimes( func: () => Promise | T, failFunc?: () => Promise | T, - repeatCount = 5, + repeatCount = 2, delayBetweenAttempts = DELAY_BETWEEN_ATTEMPTS ): Promise { try { @@ -601,7 +608,7 @@ async function tryNTimes( await new Promise((resolve) => { setTimeout(() => resolve(), delayBetweenAttempts) }) - if (repeatCount === 1) { + if (repeatCount <= 1) { console.error(`No more attempts left every ${delayBetweenAttempts}`) if (failFunc) { return failFunc() @@ -614,7 +621,7 @@ async function tryNTimes( async function tryNTimesVoidReturn( func: () => Promise | void, - repeatCount = 5, + repeatCount = 2, delayBetweenAttempts = DELAY_BETWEEN_ATTEMPTS ): Promise { return tryNTimes(func, () => undefined, repeatCount, delayBetweenAttempts) diff --git a/src/server/gsheets/expressAccessors.ts b/src/server/gsheets/expressAccessors.ts index 784bfde..5d498dd 100644 --- a/src/server/gsheets/expressAccessors.ts +++ b/src/server/gsheets/expressAccessors.ts @@ -9,14 +9,33 @@ export default class ExpressAccessors< ElementNoId extends object, Element extends ElementWithId > { - sheet: Sheet + sheet?: Sheet + + runAfterLoad: ((value: Sheet) => void)[] = [] + + isLoaded = false constructor( readonly sheetName: keyof SheetNames, readonly specimen: Element, readonly translation: { [k in keyof Element]: string } ) { - this.sheet = getSheet(sheetName, specimen, translation) + getSheet(this.sheetName, this.specimen, this.translation).then( + (sheet) => { + this.sheet = sheet + this.isLoaded = true + this.runAfterLoad.map((f) => f(sheet)) + } + ) + } + + async getSheet(): Promise> { + if (!this.isLoaded) { + await new Promise((resolve) => { + this.runAfterLoad.push(resolve) + }) + } + return this.sheet as Sheet } listGet() { @@ -26,7 +45,7 @@ export default class ExpressAccessors< _next: NextFunction ): Promise => { try { - const elements = await this.sheet.getList() + const elements = await (await this.getSheet()).getList() if (elements) { response.status(200).json(elements) } @@ -42,7 +61,8 @@ export default class ExpressAccessors< ) { return async (request: Request, response: Response, _next: NextFunction): Promise => { try { - const list = (await this.sheet.getList()) || [] + const sheet = await this.getSheet() + const list = (await sheet.getList()) || [] let toCaller: any if (!custom) { const id = parseInt(request.query.id as string, 10) || -1 @@ -65,7 +85,8 @@ export default class ExpressAccessors< add() { return async (request: Request, response: Response, _next: NextFunction): Promise => { try { - const element: Element = await this.sheet.add(request.body) + const sheet = await this.getSheet() + const element: Element = await sheet.add(request.body) if (element) { response.status(200).json(element) } @@ -85,15 +106,16 @@ export default class ExpressAccessors< ) { return async (request: Request, response: Response, _next: NextFunction): Promise => { try { + const sheet = await this.getSheet() if (!custom) { - await this.sheet.set(request.body) + await sheet.set(request.body) response.status(200) } else { const memberId = response?.locals?.jwt?.id || -1 - const list = (await this.sheet.getList()) || [] + const list = (await sheet.getList()) || [] const { toDatabase, toCaller } = await custom(list, request.body, memberId) if (toDatabase !== undefined) { - await this.sheet.set(toDatabase) + await sheet.set(toDatabase) } if (toCaller !== undefined) { response.status(200).json(toCaller) diff --git a/src/server/notifications.ts b/src/server/notifications.ts index 2b02d42..8ec2a28 100644 --- a/src/server/notifications.ts +++ b/src/server/notifications.ts @@ -80,7 +80,7 @@ export function notificationMain(): void { } async function notifyAboutAnnouncement(): Promise { - const announcementSheet = getSheet( + const announcementSheet = await getSheet( "Announcements", new Announcement(), translationAnnouncement @@ -96,7 +96,7 @@ async function notifyAboutAnnouncement(): Promise { return } - const volunteerSheet = getSheet( + const volunteerSheet = await getSheet( "Volunteers", new Volunteer(), translationVolunteer