[lib] don't process persisted inbound messages without `viewerID`
ClosedPublic
Actions

Authored by kamil on Sep 24 2024, 3:56 AM.

Details

Reviewers

tomek

Commits

rCOMMcfc5daf4fbc4: [lib] don't process persisted inbound messages without `viewerID`

Summary

ENG-9381.

I had some InboundP2PMessage's in the store, and I discovered that they're causing ignored DMOperation because logged out.
This is a race condition because this component can run before getting loggedInUserInfo in getInitialReduxState and useProcessDMOperation was called without viewerID.

This fixes the issue.

This is also safe for messages not from DB but received from Tunnelbroker, we're going to establish a socket connection only when viewerID is defined anyway.

Test Plan

Seeing the messages processed.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kamil created this revision.Sep 24 2024, 3:56 AM

kamil held this revision as a draft.

Herald added a subscriber: ashoat. · View Herald TranscriptSep 24 2024, 3:56 AM

kamil published this revision for review.Sep 24 2024, 3:57 AM

kamil edited the summary of this revision. (Show Details)

tomek accepted this revision.Sep 24 2024, 4:05 AM

This revision is now accepted and ready to land.Sep 24 2024, 4:05 AM

Harbormaster completed remote builds in B31812: Diff 44479.Sep 24 2024, 4:13 AM

Closed by commit rCOMMcfc5daf4fbc4: [lib] don't process persisted inbound messages without `viewerID` (authored by kamil). · Explain WhySep 25 2024, 6:00 AM

This revision was automatically updated to reflect the committed changes.

kamil added a commit: rCOMMcfc5daf4fbc4: [lib] don't process persisted inbound messages without `viewerID`.

Revision Contents
Changeset List

Path

Size

lib/

tunnelbroker/

peer-to-peer-message-handler.js

6 lines

Diff 44535

View Options

lib/tunnelbroker/peer-to-peer-message-handler.js

// @flow		// @flow

import { DATABASE_WORKER_PATH, SQLJS_FILE_PATH } from './utils/constants.js';		import { DATABASE_WORKER_PATH, SQLJS_FILE_PATH } from './utils/constants.js';
import { isSQLiteSupported } from './utils/db-utils.js';		import { isDesktopSafari, isSQLiteSupported } from './utils/db-utils.js';
import WorkerConnectionProxy from './utils/WorkerConnectionProxy.js';		import WorkerConnectionProxy from './utils/WorkerConnectionProxy.js';
import type { AppState } from '../redux/redux-setup.js';		import type { AppState } from '../redux/redux-setup.js';
import {		import {
workerRequestMessageTypes,		workerRequestMessageTypes,
type WorkerRequestMessage,		type WorkerRequestMessage,
type WorkerResponseMessage,		type WorkerResponseMessage,
} from '../types/worker-types.js';		} from '../types/worker-types.js';

Show All 16 Lines	class DatabaseModule {
status: DatabaseStatus;		status: DatabaseStatus;

constructor() {		constructor() {
const currentLoggedInUserID = preloadedState.currentUserInfo?.anonymous		const currentLoggedInUserID = preloadedState.currentUserInfo?.anonymous
? undefined		? undefined
: preloadedState.currentUserInfo?.id;		: preloadedState.currentUserInfo?.id;
const isSupported = isSQLiteSupported(currentLoggedInUserID);		const isSupported = isSQLiteSupported(currentLoggedInUserID);

if (!isSupported) {		if (!isSupported \|\| isDesktopSafari()) {
		tomekUnsubmitted Not Done Inline Actions Can we have a const instead of a function that tells if it is Safari? We can avoid having to detect the browser multiple times, as this fact doesn't change through the lifetime of the app. tomek: Can we have a const instead of a function that tells if it is Safari? We can avoid having to…
		kamilAuthorUnsubmitted Done Inline Actions good point, thanks kamil: good point, thanks
this.status = databaseStatuses.notSupported;		this.status = databaseStatuses.notSupported;
		kamilAuthorUnsubmitted Done Inline Actions Naming might be confusing, but at this point database is not supported (we don't have a key to it), it will be supported after main thread will provide encryption key kamil: Naming might be confusing, but at this point database is not supported (we don't have a key to…
		tomekUnsubmitted Not Done Inline Actions Do we change the status later? tomek: Do we change the status later?
		kamilAuthorUnsubmitted Done Inline Actions yes, in `initDBForLoggedInUser()` kamil: yes, in `initDBForLoggedInUser()`
} else {		} else {
this.init();		this.init();
}		}
}		}

init(encryptionKey?: ?SubtleCrypto$JsonWebKey) {		init(encryptionKey?: ?SubtleCrypto$JsonWebKey) {
this.status = databaseStatuses.initInProgress;		this.status = databaseStatuses.initInProgress;
this.worker = new SharedWorker(DATABASE_WORKER_PATH);		this.worker = new SharedWorker(DATABASE_WORKER_PATH);
▲ Show 20 Lines • Show All 77 Lines • Show Last 20 Lines

[lib] don't process persisted inbound messages without `viewerID`ClosedPublicActions