Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

misc push-related fixes and improvements #2649

Merged
merged 31 commits into from
Feb 22, 2025
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
b0f1067
misc fixes and improvements
taoeffect Feb 18, 2025
992d4fa
many fixes and improvements to all the notification things, mostly fr…
taoeffect Feb 19, 2025
a26169f
improve logging
taoeffect Feb 19, 2025
2e0cf69
log the subscription ID on both the frontend and backend
taoeffect Feb 19, 2025
3dfb45a
improved logging
taoeffect Feb 19, 2025
e46a06b
some bugfixes and improvements to backend/push.js
taoeffect Feb 19, 2025
fe3511c
feedback
taoeffect Feb 20, 2025
ec1601b
remove extraneous call to setupChelonia()
taoeffect Feb 20, 2025
4587e91
minor improvements
taoeffect Feb 20, 2025
9e6d83e
address all remaining issue. tested in Safari + Brave + FF
taoeffect Feb 20, 2025
a2177d2
prevent log spam
taoeffect Feb 20, 2025
a123826
oops
taoeffect Feb 20, 2025
234e050
update strings
taoeffect Feb 20, 2025
84c8a4d
possibly address #2653
taoeffect Feb 20, 2025
3684654
ensure local Cypress tests don't get tripped by by push issues
taoeffect Feb 20, 2025
bf51c3a
react to notification permission changes in Safari
taoeffect Feb 21, 2025
23a3c2f
improvement to syncContractsInOrder to prevent syncing removed contracts
taoeffect Feb 21, 2025
ccf683f
feedback
taoeffect Feb 21, 2025
1b89e4d
feedback: syncContractsInOrder should attempt to sync all contracts
taoeffect Feb 21, 2025
6c417b0
improve error message
taoeffect Feb 21, 2025
dc4c255
improve logging in syncContractsInOrder
taoeffect Feb 21, 2025
7f9e00d
webkit bugfix + throttle handler for Chrome
taoeffect Feb 21, 2025
4694034
move back the call to syncContractsInOrder where it was
taoeffect Feb 21, 2025
48e53f7
handle 'WebSocket connection is not open' on 'push/reportExistingSubs…
taoeffect Feb 21, 2025
6655820
better comments + logging
taoeffect Feb 21, 2025
8189552
improve gi.ui prompt strings
taoeffect Feb 21, 2025
d4369e7
remove redundant alert() + improve error logging
taoeffect Feb 21, 2025
f46d735
Remove Notification object on Cypress
corrideat Feb 22, 2025
03cb7ee
feedback
taoeffect Feb 22, 2025
8bedd74
created ''sw/deviceSettings/*' and used it for handling DISABLE_NOTIF…
taoeffect Feb 22, 2025
9af8fa7
final feedback
taoeffect Feb 22, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
263 changes: 109 additions & 154 deletions backend/push.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion backend/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ sbp('okTurtles.data/set', PUBSUB_INSTANCE, createServer(hapi.listener, {
await handler.call(socket, payload)
} catch (error) {
const message = error?.message || `push server failed to perform [${action}] action`
console.warn(`Handler '${REQUEST_TYPE.PUSH_ACTION}' failed: ${message}`)
console.warn(error, `Action '${action}' for '${REQUEST_TYPE.PUSH_ACTION}' handler failed: ${message}`)
socket.send(createPushErrorResponse({ actionType: action, message }))
}
} else {
Expand Down
4 changes: 4 additions & 0 deletions frontend/assets/style/components/_forms.scss
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,10 @@ p.error {
&:disabled:not(:checked)::after {
opacity: 0.6;
}

&:disabled {
background: $general_0;
}
}

.is-dark-theme {
Expand Down
27 changes: 14 additions & 13 deletions frontend/controller/actions/identity.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { SETTING_CHELONIA_STATE } from '@model/database.js'
import sbp from '@sbp/sbp'
import { imageUpload, objectURLtoBlob } from '@utils/image.js'
import { SETTING_CURRENT_USER } from '~/frontend/model/database.js'
import { JOINED_CHATROOM, KV_QUEUE, LOGIN, LOGOUT } from '~/frontend/utils/events.js'
import { JOINED_CHATROOM, KV_QUEUE, LOGIN, LOGOUT, LOGGING_OUT } from '~/frontend/utils/events.js'
import { GIMessage } from '~/shared/domains/chelonia/GIMessage.js'
import { Secret } from '~/shared/domains/chelonia/Secret.js'
import { encryptedIncomingDataWithRawKey, encryptedOutgoingData, encryptedOutgoingDataWithRawKey } from '~/shared/domains/chelonia/encryptedData.js'
Expand Down Expand Up @@ -413,15 +413,15 @@ export default (sbp('sbp/selectors/register', {

try {
if (!state) {
// Make sure we don't unsubscribe from our own identity contract
// Note that this should be done _after_ calling
// `chelonia/storeSecretKeys`: If the following line results in
// syncing the identity contract and fetching events, the secret keys
// for processing them will not be available otherwise.
// Make sure we don't unsubscribe from our own identity contract
// Note that this should be done _after_ calling
// `chelonia/storeSecretKeys`: If the following line results in
// syncing the identity contract and fetching events, the secret keys
// for processing them will not be available otherwise.
await sbp('chelonia/contract/retain', identityContractID)
} else {
// If there is a state, we've already retained the identity contract
// but might need to fetch the latest events
// If there is a state, we've already retained the identity contract
// but might need to fetch the latest events
await sbp('chelonia/contract/sync', identityContractID)
}
} catch (e) {
Expand All @@ -437,7 +437,7 @@ export default (sbp('sbp/selectors/register', {
await syncContractsInOrder(contractIDs)

try {
// The state above might be null, so we re-grab it
// The state above might be null, so we re-grab it
const cheloniaState = sbp('chelonia/rootState')

// The updated list of groups
Expand All @@ -451,8 +451,8 @@ export default (sbp('sbp/selectors/register', {
// Call 'gi.actions/group/join' on all groups which may need re-joining
await Promise.allSettled(
groupIds.map(async groupId => (
// (1) Check whether the contract exists (may have been removed
// after sync)
// (1) Check whether the contract exists (may have been removed
// after sync)
has(cheloniaState.contracts, groupId) &&
has(cheloniaState[identityContractID].groups, groupId) &&
// (2) Check whether the join process is still incomplete
Expand Down Expand Up @@ -486,8 +486,8 @@ export default (sbp('sbp/selectors/register', {
// $FlowFixMe[incompatible-use]
.filter(([, { hasLeft }]) => !hasLeft)
.forEach(([cId]) => {
// We send this action only for groups we have fully joined (i.e.,
// accepted an invite and added our profile)
// We send this action only for groups we have fully joined (i.e.,
// accepted an invite and added our profile)
if (cheloniaState[cId]?.profiles?.[identityContractID]?.status === PROFILE_STATUS.ACTIVE) {
sbp('gi.actions/group/kv/updateLastLoggedIn', { contractID: cId, throttle: false }).catch((e) => console.error('Error sending updateLastLoggedIn', e))
}
Expand Down Expand Up @@ -520,6 +520,7 @@ export default (sbp('sbp/selectors/register', {
// error occurs)
'gi.actions/identity/_private/logout': async function () {
let cheloniaState
sbp('okTurtles.events/emit', LOGGING_OUT)
try {
console.info('logging out, waiting for any events to finish...')
// wait for any pending operations to finish before calling state/vuex/save
Expand Down
31 changes: 23 additions & 8 deletions frontend/controller/actions/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ export async function createInvite ({ contractID, quantity = 1, creatorID, expir
}
}

export function groupContractsByType (contracts: Object): Object {
export function groupContractsByType (contracts?: Object): Object {
const contractIDs = Object.create(null)
if (contracts) {
// Note: `references` holds non-ephemeral references (i.e., explicit
Expand Down Expand Up @@ -357,15 +357,30 @@ export async function syncContractsInOrder (groupedContractIDs: Object): Promise
const index = contractSyncPriorityList.indexOf(key)
return index === -1 ? contractSyncPriorityList.length : index
}

const failedSyncs = []
try {
// $FlowFixMe[incompatible-call]
await Promise.all(Object.entries(groupedContractIDs).sort(([a], [b]) => {
// Sync contracts in order based on type
const sortedContractTypes = Object.entries(groupedContractIDs).sort(([a], [b]) => {
return getContractSyncPriority(a) - getContractSyncPriority(b)
}).map(([, ids]) => {
return sbp('chelonia/contract/sync', ids)
}))
})
for (const [type, contractIDs] of sortedContractTypes) {
// For each contract of this type, check if it still exists before syncing because
// e.g. syncing a group contract could have removed one of the chatroom contracts
// $FlowFixMe[incompatible-type]
for (const contractID of contractIDs) {
const { contracts } = sbp('chelonia/rootState')
if (contractID in contracts) {
try {
await sbp('chelonia/contract/sync', contractID)
} catch (e) {
console.error(`syncContractsInOrder: failed to sync ${type}(${contractID}):`, e)
failedSyncs.push(`${type}(…${contractID.slice(-5)}) failed sync with '${e.message}'`)
}
} else {
console.warn(`syncContractsInOrder: skipping ${type}(${contractID}) as it was removed while syncing previous contracts`)
}
}
}
if (failedSyncs.length > 0) throw new Error(failedSyncs.join(', '))
} catch (err) {
console.error('Error during contract sync (syncing all contractIDs)', err)
throw err
Expand Down
6 changes: 3 additions & 3 deletions frontend/controller/app/identity.js
Original file line number Diff line number Diff line change
Expand Up @@ -349,9 +349,9 @@ export default (sbp('sbp/selectors/register', {
const loginCompleteHandler = ({ identityContractID: id }) => {
removeLoginErrorHandler()
if (id === identityContractID) {
// Before the promise resolves, we need to save the state
// by calling 'state/vuex/save' to ensure that refreshing the page
// results in a page with the same state.
// Before the promise resolves, we need to save the state
// by calling 'state/vuex/save' to ensure that refreshing the page
// results in a page with the same state.
resolve(sbp('state/vuex/save'))
} else {
reject(new Error(`Identity contract ID mismatch during login: ${identityContractID} != ${id}`))
Expand Down
139 changes: 69 additions & 70 deletions frontend/controller/service-worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { HOURS_MILLIS } from '~/frontend/model/contracts/shared/time.js'
import { GIMessage } from '~/shared/domains/chelonia/GIMessage.js'
import { Secret } from '~/shared/domains/chelonia/Secret.js'
import { deserializer, serializer } from '~/shared/serdes/index.js'
import { ONLINE } from '../utils/events.js'
import { getSubscriptionId } from '~/shared/functions.js'

const pwa = {
deferredInstallPrompt: null,
Expand Down Expand Up @@ -183,72 +183,83 @@ sbp('sbp/selectors/register', {
// In theory, the `PushManager` APIs used are available in the SW and we could
// have this function there. However, most examples perform this outside of the
// SW, and private testing showed that it's more reliable doing it here.
'service-worker/setup-push-subscription': async function (retryCount?: number) {
if (process.env.CI) {
'service-worker/setup-push-subscription': async function (attemptNumber = 1) {
if (window.Cypress || process.env.CI) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a strong feeling either way, but the reason I was not checking for window.Cypress (and generally avoid doing so) is that these are runtime checks (code shipped) while process.env.CI is exclusively a buildtime check (no code shipped).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to add it because without it the tests weren't passing. An alternative would be to create a Cypress envvar in Gruntfile.js...

throw new Error('Push disabled in CI mode')
}
let retryAttemptsPromise = null
await sbp('okTurtles.eventQueue/queueEvent', 'service-worker/setup-push-subscription', async () => {
const { notificationEnabled } = sbp('state/vuex/state').settings
// Get the installed service-worker registration
const registration = await navigator.serviceWorker.ready

if (!registration) {
console.error('No service-worker registration found!')
return
throw new Error('No service-worker registration found!')
}

// Safari sometimes incorrectly reports 'prompt' when using `registration.pushManager.permissionState`
const permissionState = await registration.pushManager.permissionState({ userVisibleOnly: true })
const granted = permissionState === 'granted' || Notification.permission === 'granted'
console.info(`[service-worker/setup-push-subscription] setup: notifications (${notificationEnabled}) perms (${granted})`)

// Safari sometimes incorrectly reports 'prompt' when using
// `registration.pushManager.permissionState`.
const existingSubscription = permissionState === 'granted' || Notification.permission === 'granted'
? await registration.pushManager.getSubscription().then((subscription) => {
if (
!subscription ||
(subscription.expirationTime != null &&
subscription.expirationTime <= Date.now())
) {
console.info(
'Attempting to create a new subscription',
subscription
)
return sbp('push/getSubscriptionOptions').then(function (options) {
return registration.pushManager.subscribe(options)
})
let subscription = null
// report a real push subscription only if both browser permissions allow and user wants us to
if (notificationEnabled && granted) {
try {
subscription = await registration.pushManager.getSubscription()
let newSub = false
let endpoint = null
let subID = null
if (!subscription || (subscription.expirationTime != null && subscription.expirationTime <= Date.now())) {
subscription = await registration.pushManager.subscribe(await sbp('push/getSubscriptionOptions'))
newSub = true
}
if (subscription?.endpoint) {
endpoint = new URL(subscription.endpoint).host // hide the full endpoint from the logs for privacy
subID = await getSubscriptionId(subscription.toJSON())
}
return subscription
}).catch(e => {
if (!(retryCount > 3) && e?.message === 'WebSocket connection is not open') {
return new Promise((resolve, reject) => {
const errorTimeoutId = setTimeout(() => {
reject(new Error('Timed out waiting to come back online'))
cancel()
}, 600e3)
const cancel = sbp('okTurtles.events/once', ONLINE, () => {
clearTimeout(errorTimeoutId)
setTimeout(() => resolve(sbp('service-worker/setup-push-subscription', (retryCount || 0) + 1)), 200)
})
console.info(`[service-worker/setup-push-subscription] got ${newSub ? 'new' : 'existing'} subscription '${subID}':`, endpoint)
} catch (e) {
console.error('[service-worker/setup-push-subscription] error getting a subscription:', e)
if (e?.message === 'WebSocket connection is not open') {
if (attemptNumber >= 3) {
console.error('[service-worker/setup-push-subscription] maxAttempts reached, giving up')
throw e // give up
}
// this outer promise is a way to wait on this sub-call to finish with getting the eventQueue stuck
retryAttemptsPromise = new Promise((resolve, reject) => {
// try again in 1 second
setTimeout(() => {
sbp('service-worker/setup-push-subscription', attemptNumber + 1).then(resolve).catch(reject)
}, 1e3)
})
return // exit the event queue immediately and do not rethrow
} else {
sbp('gi.ui/prompt', {
heading: L('Error setting up push notifications'),
question: L('Error setting up push notifications: {errMsg}{br_}{br_}Please make sure {a_}push services are enabled{_a} in your Browser settings, and then try toggling the push notifications toggle in the Notifications settings in the app to try again.', {
errMsg: e?.message,
a_: '<a class="link" target="_blank" href="https://stackoverflow.com/a/69624651">',
_a: '</a>',
br_: '<br/>'
}),
primaryButton: L('Close')
})
}
console.error('[service-worker/setup-push-subscription] Error setting up push subscription', e)
sbp('gi.ui/prompt', {
heading: L('Error setting up push notifications'),
question: L('Error setting up push notifications: {errMsg}{br_}{br_}Please make sure {a_}push services are enabled{_a} in your Browser settings, and then try toggling the push notifications toggle in the Notifications settings in the app to try again.', {
errMsg: e?.message,
a_: '<a class="link" target="_blank" href="https://stackoverflow.com/a/69624651">',
_a: '</a>',
br_: '<br/>'
}),
primaryButton: L('Close')
})
throw e
})
: null

await sbp('push/reportExistingSubscription', existingSubscription?.toJSON()).catch(e => {
console.error('[service-worker/setup-push-subscription] Error reporting existing subscription', e)
})
return true
}
}
try {
console.info('[service-worker/setup-push-subscription] calling push/reportExistingSubscription...')
await sbp('push/reportExistingSubscription', subscription?.toJSON())
} catch (e) {
console.error('[service-worker/setup-push-subscription] error reporting subscription:', e)
throw e
}
})
if (retryAttemptsPromise) {
// wait until all attempts have finished
await retryAttemptsPromise
}
},
'service-worker/update': async function () {
// This function manually checks for the service worker updates and trigger them if there are.
Expand Down Expand Up @@ -359,26 +370,14 @@ const swRpc = (() => {
})()

sbp('sbp/selectors/register', {
'gi.actions/*': swRpc
})
sbp('sbp/selectors/register', {
'chelonia/*': swRpc
})
sbp('sbp/selectors/register', {
'gi.actions/*': swRpc,
'chelonia/*': swRpc,
'sw-namespace/*': (...args) => {
// Remove the `sw-` prefix from the selector
return swRpc(args[0].slice(3), ...args.slice(1))
}
})
sbp('sbp/selectors/register', {
'gi.notifications/*': swRpc
})
sbp('sbp/selectors/register', {
'sw/*': swRpc
})
sbp('sbp/selectors/register', {
'swLogs/*': swRpc
})
sbp('sbp/selectors/register', {
},
'gi.notifications/*': swRpc,
'sw/*': swRpc,
'swLogs/*': swRpc,
'push/*': swRpc
})
Loading