From 42bea0264c8bb8ee1132812bf65991ec2d78670c Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 25 Apr 2024 16:28:41 +1000 Subject: [PATCH] fix: improve retrieve timeout to 10s also: - add comments about not adding the limit:256 on snode list fetch - fix an issue when no audio are found when starting a webrtc call --- ts/components/SessionInboxView.tsx | 21 +++++----------- .../dialog/EditProfilePictureModal.tsx | 1 - .../settings/SessionSettingListItem.tsx | 1 - ts/session/apis/seed_node_api/SeedNodeAPI.ts | 3 +++ .../apis/snode_api/SnodeRequestTypes.ts | 3 +++ .../apis/snode_api/getServiceNodesList.ts | 3 +++ ts/session/apis/snode_api/onions.ts | 2 +- ts/session/apis/snode_api/retrieveRequest.ts | 24 +++++++------------ ts/session/utils/calling/CallManager.ts | 1 + ts/util/storage.ts | 11 ++++++++- 10 files changed, 36 insertions(+), 34 deletions(-) diff --git a/ts/components/SessionInboxView.tsx b/ts/components/SessionInboxView.tsx index 798ac095f..b4ff71b00 100644 --- a/ts/components/SessionInboxView.tsx +++ b/ts/components/SessionInboxView.tsx @@ -1,4 +1,4 @@ -import { fromPairs, isBoolean, map } from 'lodash'; +import { fromPairs, map } from 'lodash'; import moment from 'moment'; import React from 'react'; import { Provider } from 'react-redux'; @@ -87,13 +87,6 @@ function createSessionInboxStore() { return createStore(initialState); } -function getBoolFromStorageOrFalse(settingsKey: string): boolean { - const got = Storage.get(settingsKey, false); - if (isBoolean(got)) { - return got; - } - return false; -} function setupLeftPane(forceUpdateInboxComponent: () => void) { window.openConversationWithMessages = openConversationWithMessages; @@ -101,15 +94,13 @@ function setupLeftPane(forceUpdateInboxComponent: () => void) { window.inboxStore.dispatch( updateAllOnStorageReady({ - hasBlindedMsgRequestsEnabled: getBoolFromStorageOrFalse( + hasBlindedMsgRequestsEnabled: Storage.getBoolOrFalse( SettingsKey.hasBlindedMsgRequestsEnabled ), - someDeviceOutdatedSyncing: getBoolFromStorageOrFalse(SettingsKey.someDeviceOutdatedSyncing), - settingsLinkPreview: getBoolFromStorageOrFalse(SettingsKey.settingsLinkPreview), - hasFollowSystemThemeEnabled: getBoolFromStorageOrFalse( - SettingsKey.hasFollowSystemThemeEnabled - ), - hasShiftSendEnabled: getBoolFromStorageOrFalse(SettingsKey.hasShiftSendEnabled), + someDeviceOutdatedSyncing: Storage.getBoolOrFalse(SettingsKey.someDeviceOutdatedSyncing), + settingsLinkPreview: Storage.getBoolOrFalse(SettingsKey.settingsLinkPreview), + hasFollowSystemThemeEnabled: Storage.getBoolOrFalse(SettingsKey.hasFollowSystemThemeEnabled), + hasShiftSendEnabled: Storage.getBoolOrFalse(SettingsKey.hasShiftSendEnabled), }) ); forceUpdateInboxComponent(); diff --git a/ts/components/dialog/EditProfilePictureModal.tsx b/ts/components/dialog/EditProfilePictureModal.tsx index a92de7a60..7828d951b 100644 --- a/ts/components/dialog/EditProfilePictureModal.tsx +++ b/ts/components/dialog/EditProfilePictureModal.tsx @@ -60,7 +60,6 @@ const uploadProfileAvatar = async (scaledAvatarUrl: string | null) => { } }; - export const EditProfilePictureModal = (props: EditProfilePictureModalProps) => { const dispatch = useDispatch(); diff --git a/ts/components/settings/SessionSettingListItem.tsx b/ts/components/settings/SessionSettingListItem.tsx index ca99d3a31..c2b111b23 100644 --- a/ts/components/settings/SessionSettingListItem.tsx +++ b/ts/components/settings/SessionSettingListItem.tsx @@ -11,7 +11,6 @@ import { SessionToggle } from '../basic/SessionToggle'; import { SessionConfirmDialogProps } from '../dialog/SessionConfirm'; import { SessionIconButton } from '../icon'; - type ButtonSettingsProps = { title?: string; description?: string; diff --git a/ts/session/apis/seed_node_api/SeedNodeAPI.ts b/ts/session/apis/seed_node_api/SeedNodeAPI.ts index 3a31aedff..a28f03eb2 100644 --- a/ts/session/apis/seed_node_api/SeedNodeAPI.ts +++ b/ts/session/apis/seed_node_api/SeedNodeAPI.ts @@ -230,6 +230,9 @@ async function getSnodesFromSeedUrl(urlObj: URL): Promise> { const params = { active_only: true, + // If you are thinking of adding the `limit` field here: don't. + // We fetch the full list because when we retrieve it we also remove from all the swarms we already know, any snode not part of that fetched list. + // If the limit was set, we would remove a lot of valid snodes from the swarms we've already fetched. fields: { public_ip: true, storage_port: true, diff --git a/ts/session/apis/snode_api/SnodeRequestTypes.ts b/ts/session/apis/snode_api/SnodeRequestTypes.ts index 1bed2be22..8b54cfd7f 100644 --- a/ts/session/apis/snode_api/SnodeRequestTypes.ts +++ b/ts/session/apis/snode_api/SnodeRequestTypes.ts @@ -73,6 +73,9 @@ export type GetServiceNodesSubRequest = { endpoint: 'get_service_nodes'; params: { active_only: true; + // If you are thinking of adding the `limit` field here: don't. + // We fetch the full list because when we retrieve it we also remove from all the swarms we already know, any snode not part of that fetched list. + // If the limit was set, we would remove a lot of valid snodes from the swarms we've already fetched. fields: { public_ip: true; storage_port: true; diff --git a/ts/session/apis/snode_api/getServiceNodesList.ts b/ts/session/apis/snode_api/getServiceNodesList.ts index 8f35d8288..6c85050a3 100644 --- a/ts/session/apis/snode_api/getServiceNodesList.ts +++ b/ts/session/apis/snode_api/getServiceNodesList.ts @@ -13,6 +13,9 @@ function buildSnodeListRequests(): Array { endpoint: 'get_service_nodes', params: { active_only: true, + // If you are thinking of adding the `limit` field here: don't. + // We fetch the full list because when we retrieve it we also remove from all the swarms we already know, any snode not part of that fetched list. + // If the limit was set, we would remove a lot of valid snodes from the swarms we've already fetched. fields: { public_ip: true, storage_port: true, diff --git a/ts/session/apis/snode_api/onions.ts b/ts/session/apis/snode_api/onions.ts index 4f2c1750a..9aedbea6d 100644 --- a/ts/session/apis/snode_api/onions.ts +++ b/ts/session/apis/snode_api/onions.ts @@ -11,7 +11,7 @@ import { AbortSignal as AbortSignalNode } from 'node-fetch/externals'; import { dropSnodeFromSnodePool, dropSnodeFromSwarmIfNeeded, updateSwarmFor } from './snodePool'; import { OnionPaths } from '../../onions'; -import { incrementBadPathCountOrDrop } from '../../onions/onionPath'; +import { incrementBadPathCountOrDrop } from '../../onions/onionPath'; import { ed25519Str, toHex } from '../../utils/String'; import { Snode } from '../../../data/data'; diff --git a/ts/session/apis/snode_api/retrieveRequest.ts b/ts/session/apis/snode_api/retrieveRequest.ts index 5dfcaf007..53f078eb0 100644 --- a/ts/session/apis/snode_api/retrieveRequest.ts +++ b/ts/session/apis/snode_api/retrieveRequest.ts @@ -1,4 +1,4 @@ -import { isArray, omit, sortBy } from 'lodash'; +import { isArray, omit } from 'lodash'; import { Snode } from '../../../data/data'; import { updateIsOnline } from '../../../state/ducks/onion'; import { doSnodeBatchRequest } from './batchRequest'; @@ -124,7 +124,7 @@ async function retrieveNextMessages( ); // let exceptions bubble up // no retry for this one as this a call we do every few seconds while polling for messages - const timeOutMs = 10 * 1000; // yes this is a long timeout for just messages, but 4s timeout way to often... + const timeOutMs = 10 * 1000; // yes this is a long timeout for just messages, but 4s timeouts way to often... const timeoutPromise = async () => sleepFor(timeOutMs); const fetchPromise = async () => doSnodeBatchRequest(retrieveRequestsParams, targetNode, timeOutMs, associatedWith); @@ -166,19 +166,13 @@ async function retrieveNextMessages( GetNetworkTime.handleTimestampOffsetFromNetwork('retrieve', bodyFirstResult.t); - // merge results with their corresponding namespaces - return results.map((result, index) => { - const messages = result.body as RetrieveMessagesResultsContent; - // Not sure if that makes sense, but we probably want those messages sorted. - const sortedMessages = sortBy(messages.messages, m => m.timestamp); - messages.messages = sortedMessages; - - return { - code: result.code, - messages, - namespace: namespaces[index], - }; - }); + // NOTE: We don't want to sort messages here because the ordering depends on the snode and when it received each messages. + // The last_hash for that snode has to be the last one we've received from that same snode, othwerwise we end up fetching the same messages over and over again. + return results.map((result, index) => ({ + code: result.code, + messages: result.body as RetrieveMessagesResultsContent, + namespace: namespaces[index], + })); } catch (e) { window?.log?.warn('exception while parsing json of nextMessage:', e); if (!window.inboxStore?.getState().onionPaths.isOnline) { diff --git a/ts/session/utils/calling/CallManager.ts b/ts/session/utils/calling/CallManager.ts index b58406297..0c04cdaf7 100644 --- a/ts/session/utils/calling/CallManager.ts +++ b/ts/session/utils/calling/CallManager.ts @@ -415,6 +415,7 @@ async function createOfferAndSendIt(recipient: string, msgIdentifier: string | n if (offer && offer.sdp) { const lines = offer.sdp.split(/\r?\n/); const lineWithFtmpIndex = lines.findIndex(f => f.startsWith('a=fmtp:111')); + // If webrtc does not find any audio input when initializing, the offer will not have a line with `a=fmtp:111` at all, `lineWithFtmpIndex` will be invalid. if (lineWithFtmpIndex > -1) { const partBeforeComma = lines[lineWithFtmpIndex].split(';'); lines[lineWithFtmpIndex] = `${partBeforeComma[0]};cbr=1`; diff --git a/ts/util/storage.ts b/ts/util/storage.ts index ba229323c..e40dac4e9 100644 --- a/ts/util/storage.ts +++ b/ts/util/storage.ts @@ -160,4 +160,13 @@ export async function saveRecentReations(reactions: Array) { return Storage.put('recent_reactions', reactions.join(' ')); } -export const Storage = { fetch, put, get, remove, onready, reset }; + +function getBoolOrFalse(settingsKey: string): boolean { + const got = Storage.get(settingsKey, false); + if (isBoolean(got)) { + return got; + } + return false; +} + +export const Storage = { fetch, put, get, getBoolOrFalse, remove, onready, reset };