From 82a1a5fc1e68bf77a14bee449755aee972218749 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Mon, 22 Nov 2021 18:37:44 +0100 Subject: [PATCH] Retry loading map on variable error If the back is getting an error (because the user has no right to set a variable), instead of failing directly, let's try to reload the map (maybe we have cached a wrong version of the map). --- back/src/Model/GameRoom.ts | 72 ++++++++++++++++++++------- back/src/Services/VariableError.ts | 9 ++++ back/src/Services/VariablesManager.ts | 7 ++- 3 files changed, 67 insertions(+), 21 deletions(-) create mode 100644 back/src/Services/VariableError.ts diff --git a/back/src/Model/GameRoom.ts b/back/src/Model/GameRoom.ts index 74849fe0..546214b3 100644 --- a/back/src/Model/GameRoom.ts +++ b/back/src/Model/GameRoom.ts @@ -26,6 +26,7 @@ import { VariablesManager } from "../Services/VariablesManager"; import { ADMIN_API_URL } from "../Enum/EnvironmentVariable"; import { LocalUrlError } from "../Services/LocalUrlError"; import { emitErrorOnRoomSocket } from "../Services/MessageHelpers"; +import { VariableError } from "../Services/VariableError"; export type ConnectCallback = (user: User, group: Group) => void; export type DisconnectCallback = (user: User, group: Group) => void; @@ -336,30 +337,61 @@ export class GameRoom { // First, let's check if "user" is allowed to modify the variable. const variableManager = await this.getVariableManager(); - const readableBy = variableManager.setVariable(name, value, user); + try { + const readableBy = variableManager.setVariable(name, value, user); - // If the variable was not changed, let's not dispatch anything. - if (readableBy === false) { - return; - } + // If the variable was not changed, let's not dispatch anything. + if (readableBy === false) { + return; + } - // TODO: should we batch those every 100ms? - const variableMessage = new VariableWithTagMessage(); - variableMessage.setName(name); - variableMessage.setValue(value); - if (readableBy) { - variableMessage.setReadableby(readableBy); - } + // TODO: should we batch those every 100ms? + const variableMessage = new VariableWithTagMessage(); + variableMessage.setName(name); + variableMessage.setValue(value); + if (readableBy) { + variableMessage.setReadableby(readableBy); + } - const subMessage = new SubToPusherRoomMessage(); - subMessage.setVariablemessage(variableMessage); + const subMessage = new SubToPusherRoomMessage(); + subMessage.setVariablemessage(variableMessage); - const batchMessage = new BatchToPusherRoomMessage(); - batchMessage.addPayload(subMessage); + const batchMessage = new BatchToPusherRoomMessage(); + batchMessage.addPayload(subMessage); - // Dispatch the message on the room listeners - for (const socket of this.roomListeners) { - socket.write(batchMessage); + // Dispatch the message on the room listeners + for (const socket of this.roomListeners) { + socket.write(batchMessage); + } + } catch (e) { + if (e instanceof VariableError) { + // Ok, we have an error setting a variable. Either the user is trying to hack the map... or the map + // is not up to date. So let's try to reload the map from scratch. + if (this.variableManagerLastLoad === undefined) { + throw e; + } + const lastLoaded = new Date().getTime() - this.variableManagerLastLoad.getTime(); + if (lastLoaded < 10000) { + console.log( + 'An error occurred while setting the "' + + name + + "\" variable. But we tried to reload the map less than 10 seconds ago, so let's fail." + ); + // Do not try to reload if we tried to reload less than 10 seconds ago. + throw e; + } + + // Reset the variable manager + this.variableManagerPromise = undefined; + + console.log( + 'An error occurred while setting the "' + name + "\" variable. Let's reload the map and try again" + ); + // Try to set the variable again! + await this.setVariable(name, value, user); + } else { + throw e; + } } } @@ -449,11 +481,13 @@ export class GameRoom { } private variableManagerPromise: Promise | undefined; + private variableManagerLastLoad: Date | undefined; private getVariableManager(): Promise { if (!this.variableManagerPromise) { this.variableManagerPromise = this.getMap() .then((map) => { + this.variableManagerLastLoad = new Date(); const variablesManager = new VariablesManager(this.roomUrl, map); return variablesManager.init(); }) diff --git a/back/src/Services/VariableError.ts b/back/src/Services/VariableError.ts new file mode 100644 index 00000000..183fab2c --- /dev/null +++ b/back/src/Services/VariableError.ts @@ -0,0 +1,9 @@ +/** + * Errors related to variable handling. + */ +export class VariableError extends Error { + constructor(message: string) { + super(message); + Object.setPrototypeOf(this, VariableError.prototype); + } +} diff --git a/back/src/Services/VariablesManager.ts b/back/src/Services/VariablesManager.ts index 915c6c05..00aac3dc 100644 --- a/back/src/Services/VariablesManager.ts +++ b/back/src/Services/VariablesManager.ts @@ -10,6 +10,7 @@ import { import { User } from "_Model/User"; import { variablesRepository } from "./Repository/VariablesRepository"; import { redisClient } from "./RedisClient"; +import { VariableError } from "./VariableError"; interface Variable { defaultValue?: string; @@ -174,11 +175,13 @@ export class VariablesManager { if (this.variableObjects) { variableObject = this.variableObjects.get(name); if (variableObject === undefined) { - throw new Error('Trying to set a variable "' + name + '" that is not defined as an object in the map.'); + throw new VariableError( + 'Trying to set a variable "' + name + '" that is not defined as an object in the map.' + ); } if (variableObject.writableBy && !user.tags.includes(variableObject.writableBy)) { - throw new Error( + throw new VariableError( 'Trying to set a variable "' + name + '". User "' +