Skip to content

Commit

Permalink
fix(connect): releasing device on browser reload using sendBeacon api
Browse files Browse the repository at this point in the history
  • Loading branch information
mroz22 committed Feb 11, 2025
1 parent 246db18 commit 9a35c82
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 5 deletions.
4 changes: 4 additions & 0 deletions packages/connect-web/src/module/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,7 @@ const TrezorConnect = factory(

export default TrezorConnect;
export * from '@trezor/connect/src/exports';

window.addEventListener('beforeunload', () => {
impl.dispose();
});
8 changes: 5 additions & 3 deletions packages/connect/src/device/Device.ts
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,7 @@ export class Device extends TypedEmitter<DeviceEvents> {
// note 3: in 99% of cases we send this message unnecessarily. as @Szymon pointed out, it might be better to catch this call and repeat it.
// note 4: this case can happen also in the 'if' branch. 1] reload app, 2], browser doesn't fire release in time, 3] you get unacquired device, 4] you click
// the 'use device here' button and here you go. Yet I didn't want to burden every TrezorConnect method call with this but we may reconsider this.
// note 5: ad note 4. it is not so problematic anymore since cleanup on dispose has been improved in https://github.com/trezor/trezor-suite/pull/16930
if (['v1', 'bridge'].includes(this.protocol.name)) {
_log.debug(
'sending a preventive cancel on the first encounter with the device',
Expand Down Expand Up @@ -1170,12 +1171,13 @@ export class Device extends TypedEmitter<DeviceEvents> {
return null;
}

async dispose() {
dispose() {
this.removeAllListeners();
if (this.session && this.lastAcquiredHere) {
try {
await this.cancelableAction?.();
await this.commands?.cancel();
// note for review: these deleted lines might not be needed anymore - in case of reload we don't have enough time
// anyway, so seems to be enough to release session so that we don't get 'unacquired' device after reloading. If
// anything stays on the screen, it will get flushed away by the initial Cancel (see device run method)

return this.transport.release({
session: this.session,
Expand Down
4 changes: 2 additions & 2 deletions packages/connect/src/device/DeviceList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -579,10 +579,10 @@ export class DeviceList extends TypedEmitter<DeviceListEvents> implements IDevic
await Promise.all(
devices.map(async device => {

Check failure on line 580 in packages/connect/src/device/DeviceList.ts

View workflow job for this annotation

GitHub Actions / Linting and formatting

Async arrow function has no 'await' expression
this.authPenaltyManager.remove(device); // TODO is this right?
await device.dispose();

return device.dispose();
}),
);

// now we can be relatively sure that release calls have been dispatched
// and we can safely kill all async subscriptions in transport layer
transport?.stop();
Expand Down
6 changes: 6 additions & 0 deletions packages/transport/src/transports/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,12 @@ export class BridgeTransport extends AbstractTransport {
onClose,
signal,
}: AbstractTransportMethodParams<'release'>) {
if (onClose && typeof navigator !== 'undefined' && 'sendBeacon' in navigator) {
navigator.sendBeacon(`${this.url}/release/${session}?beacon=1`);

return Promise.resolve(this.success(null));
}

return this.scheduleAction(
async signal => {
const releasePromise = this.post('/release', {
Expand Down

0 comments on commit 9a35c82

Please sign in to comment.