michael/cordis
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

fixed bug with verification

Browse Source 
Fixes problem with guildMeta not updating from the server
Fixes problem with unknown member for "second" guild

The pairVerifier was getting unverified due to the socket connecting
*after* the fetch request was sent. Unfortunately, this unverification
happened *before* the fetch request was resolved. Fixed by re-fetching
the trusted promise after unverification. For a perfect solution in the
future, we should remove the re-fetch. This could be done by canceling
socket-guild fetches if the socket isn't connected yet.
This commit is contained in:
Michael Peters 2022-02-03 18:06:26 -06:00
parent 222feb1de1
commit 09390fad8f
4 changed files with 51 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
src/client/webapp/auto-verifier.ts
View File

@ -1,10 +1,12 @@
// import * as electronRemote from '@electron/remote';
// const electronConsole = electronRemote.getGlobal('console') as Console;
// import Logger from '../../logger/logger';
// const LOG = Logger.create(__filename, electronConsole);
import * as electronRemote from '@electron/remote';
const electronConsole = electronRemote.getGlobal('console') as Console;
import Logger from '../../logger/logger';
const LOG = Logger.create(__filename, electronConsole);
import { Changes, WithEquals } from './data-types';
import * as uuid from 'uuid';
export enum AutoVerifierChangesType {
NONE, // Both primaryFunc and trustedFunc returned null
PRIMARY_ONLY, // primaryFunc returned non-null and trustedFunc returned null
@ -14,17 +16,19 @@ export enum AutoVerifierChangesType {
}
/**
* This is probably complex piece of code in this entire project.
* This is probably the most complex piece of code in this entire project.
* Some complexity comes because the result of the trusted function
* can be invalidated while we are awaiting the trusted function.
* That complexity stacks on top of the already complex cache-verification
* to make this one fiesta of a class.
* If you have to edit this it's a very sad day.
* If you have to edit this it's going to be a "learning experience"
*/
export class AutoVerifier<T> {
public primaryPromise: Promise<T | null> | null = null;
public trustedPromise: Promise<T | null> | null = null;
public trustedStatus: 'none' | 'fetching' | 'verifying' | 'verified' = 'none';
private verifierId: string;
/**
* Allows a trusted function to verify the primary function
@ -36,7 +40,9 @@ export class AutoVerifier<T> {
private primaryFunc: () => Promise<T | null>,
private trustedFunc: () => Promise<T | null>,
private verifyFunc: (primaryResult: T | null, trustedResult: T | null) => Promise<boolean>
) {}
) {
this.verifierId = uuid.v4();
}
/** Returns the changes that must be made to primaryResult given trustedResult */
static getChanges<T extends WithEquals<T> & { id: string }>(primaryResult: T[] | null, trustedResult: T[] | null): Changes<T> {
@ -151,18 +157,22 @@ export class AutoVerifier<T> {
// If the primary fetchable returns null but has not been verified yet, this will return the result of the trusted fetchable
// If the trusted fetchable has not been used to verify the primary fetchable yet, this queries the trusted fetchable and calls verify
// @param lazyVerify: set to true to only verify if primaryResult returns null (useful for fetching resources since they can never change)
async fetchAndVerifyIfNeeded(lazyVerify = false): Promise<T | null> {
// @param debug: print debug messages. This is useful if you (unfortunately) think there is a bug in this
async fetchAndVerifyIfNeeded(lazyVerify = false, debug = false): Promise<T | null> {
// eslint-disable-next-line no-async-promise-executor
return await new Promise<T | null>(async (resolve: (result: T | null) => void, reject: (error: Error) => void) => {
let resolved = false;
const fetchId = debug && `v#${this.verifierId} f#${uuid.v4()}`;
try {
if (this.primaryPromise === null) {
if (debug) LOG.debug(fetchId + ': created primary promise');
this.primaryPromise = this.primaryFunc();
}
const origPrimaryPromise = this.primaryPromise;
// pre-fetch the trusted result while we fetch the primary result
if (!lazyVerify && this.trustedStatus === 'none') {
if (debug) LOG.debug(fetchId + ': created trusted promise, set to \'fetching\'');
this.trustedStatus = 'fetching';
this.trustedPromise = this.trustedFunc();
}
@ -170,19 +180,23 @@ export class AutoVerifier<T> {
const primaryResult = await this.primaryPromise;
if (this.primaryPromise === origPrimaryPromise) {
// Reset the primary promise so we create a new one next time
if (debug) LOG.debug('reset the primary promise for next time');
this.primaryPromise = null;
}
if (primaryResult) {
if (debug) LOG.debug('resolving with primary result');
resolve(primaryResult);
resolved = true;
if (lazyVerify || this.trustedStatus === 'verified') {
if (debug) LOG.debug(fetchId + ': not waiting on trusted promise', { lazyVerify, trustedStatus: this.trustedStatus });
return;
}
}
if (this.trustedStatus === 'none') {
// try to re-fetch the trusted result
if (debug) LOG.debug(fetchId + ': creating trusted result (since status is \'none\'');
this.trustedStatus = 'fetching';
this.trustedPromise = this.trustedFunc();
}
@ -195,6 +209,7 @@ export class AutoVerifier<T> {
// No one has started verifying the trusted yet
this.trustedStatus = 'verifying';
if (debug) LOG.debug(fetchId + ': verifying... (awaiting trustedPromise)');
// Note: Promises that have already resolved will return the same value when awaited again :)
const origTrustedPromise: Promise<T | null> | null = this.trustedPromise;
@ -202,7 +217,15 @@ export class AutoVerifier<T> {
if (this.trustedPromise !== origTrustedPromise) {
// we've been invalidated while we were waiting for the trusted result!
console.warn('RARE ALERT: we got unverified while trying to fetch a trusted promise for verification!');
// TODO: This happens when a socket fetch is sent before the socket is connected to.
if (debug) LOG.debug(fetchId + ': unverified during fetch!', new Error());
if (debug) LOG.debug(fetchId + ': trustedPromise now: ', { trustedPromise: this.trustedPromise })
console.warn('RARE ALERT: we got unverified while trying to fetch a trusted promise for verification!', new Error());
if (this.trustedPromise === null) {
if (debug) LOG.debug(fetchId + ': re-fetching since trustedPromise was null');
this.trustedStatus = 'fetching';
this.trustedPromise = this.trustedFunc();
}
await tryResolveTrustedPromise();
return;
}
@ -215,9 +238,11 @@ export class AutoVerifier<T> {
if (trustedResult !== null && primaryUpToDate) {
// We got a good trusted result and the primary data has been updated
// to reflect the trusted data (or already reflects it).
if (debug) LOG.debug(fetchId + ': verified successfully.');
this.trustedStatus = 'verified';
} else {
// We have to re-fetch the trusted promise again next fetch
if (debug) LOG.debug(fetchId + ': needs trusted promise re-fetched next time');
this.trustedStatus = 'none';
}
} else {
@ -228,6 +253,7 @@ export class AutoVerifier<T> {
}
if (!resolved) { // Removed 01/09/2021 pretty sure should not be here... && trustedResult
if (debug) LOG.debug(fetchId + ': resolving with trusted result');
resolve(trustedResult);
resolved = true;
}
@ -235,6 +261,8 @@ export class AutoVerifier<T> {
// Some code is already dealing with (or dealt with) verifying the trusted result
// Await the same trusted promise and return its result if we didn't get a result
// from the primary source.
if (debug) LOG.debug(fetchId + ': waiting on result of a different verifier...');
// Note: Promises that have already resolved will return the same value when awaited again :)
const origTrustedPromise: Promise<T | null> | null = this.trustedPromise;
@ -242,12 +270,14 @@ export class AutoVerifier<T> {
if (this.trustedPromise !== origTrustedPromise) {
// we've been invalidated while we were waiting for the trusted result!
if (debug) LOG.debug(fetchId + ': got unverified while waiting on the result of a different verifier!', new Error());
console.warn('ULTRA RARE ALERT: we got unverified while awaiting a trusted promise another path was verifying!');
await tryResolveTrustedPromise();
return;
}
if (!resolved) {
if (debug) LOG.debug(fetchId + ': resolving with trusted result of different verifier');
resolve(trustedResult);
resolved = true;
}
@ -255,6 +285,7 @@ export class AutoVerifier<T> {
} else {
// we are all up to date, make sure to resolve if primaryResult is null
if (!resolved) {
if (debug) LOG.debug(fetchId + ': no trusted promise, resolving with null');
resolve(null);
resolved = true;
}
@ -264,9 +295,11 @@ export class AutoVerifier<T> {
} catch (e: unknown) {
this.unverify();
if (!resolved) {
if (debug) LOG.debug(fetchId + ': error during fetch', e);
reject(e as Error);
resolved = true;
} else {
if (debug) LOG.debug(fetchId + ': server request failed after returning cache value (or we already rejected)', e);
console.warn('server request failed after returning cache value (or when already rejected)', e);
}
}

2
src/client/webapp/elements/lists/components/channel-element.tsx
View File

@ -28,7 +28,7 @@ const ChannelElement: FC<ChannelElementProps> = (props: ChannelElementProps) =>
const modifyRef = useRef<HTMLDivElement>(null);
const baseClassName = (isLoaded(activeChannel) && activeChannel.value.id === channel.id) ? 'channel text active' : 'channel-text';
const baseClassName = (isLoaded(activeChannel) && activeChannel.value.id === channel.id) ? 'channel text active' : 'channel text';
const [ modifyContextHover, modifyMouseEnterCallable, modifyMouseLeaveCallable ] = useContextHover(
() => {

4
src/client/webapp/elements/require/atoms-2.ts
View File

@ -534,8 +534,8 @@ function createCurrentGuildLoadableStateGetter<T>(subSelectorFamily: (guildId: n
}
}
// Note: These will all update in parallel when the guild changes. They will always reference the same guild
// There should not need to be a worry about them cauing extra renders
// Note: These will all update in parallel when the guild changes. They will always reference the same guild.
// What's great about these is that they all change at once when the guild is changed so there are not extraneous renders!
export const currGuildState = selector<CombinedGuild | null>({
key: 'currGuildState',
get: createCurrentGuildStateGetter(guildState),

8
src/server/server-controller.ts
View File

@ -327,11 +327,13 @@ function bindAdminEvents(io: socketio.Server, client: socketio.Socket, identity:
if (name.length == 0 || name.length > MAX_GUILD_NAME_LENGTH) throw new EventError('invalid guild name');
if (!identity.guildId) throw new EventError('identity no guildId');
LOG.debug(`g#${identity.guildId} u#${identity.memberId} set-name to ${name}`)
const newMeta = await DB.setName(identity.guildId, name);
respond(null, newMeta);
io.emit('update-metadata', newMeta);
io.to(identity.guildId).emit('update-metadata', newMeta);
}
);
@ -350,6 +352,8 @@ function bindAdminEvents(io: socketio.Server, client: socketio.Socket, identity:
if (iconBuff.length == 0 || iconBuff.length > MAX_ICON_SIZE) throw new EventError('invalid guild icon');
if (!identity.guildId) throw new EventError('identity no guildId');
LOG.debug(`g#${identity.guildId} u#${identity.memberId} set-icon`)
const typeResult = await FileType.fromBuffer(iconBuff);
if (!typeResult || !['image/png', 'image/jpeg', 'image/jpg'].includes(typeResult.mime)) {
throw new EventError('detected invalid mime type');
@ -360,7 +364,7 @@ function bindAdminEvents(io: socketio.Server, client: socketio.Socket, identity:
respond(null, newMeta);
io.emit('update-metadata', newMeta);
io.to(identity.guildId).emit('update-metadata', newMeta);
}
);