Skip to content

Commit 48819c6

Browse files
authored
Retry attempts to close the dialog (#274)
* When attempts to close the dialog fail, we check whether the interface exists and then try again * Make sure we cleanup after retries fail
1 parent 1da1a70 commit 48819c6

File tree

3 files changed

+143
-14
lines changed

3 files changed

+143
-14
lines changed

packages/gator-permissions-snap/src/core/dialogInterface.ts

Lines changed: 59 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -64,23 +64,73 @@ export class DialogInterface {
6464
return this.#interfaceId;
6565
}
6666

67+
/**
68+
* Tries to close the dialog interface.
69+
* @param interfaceId - The ID of the interface to attempt to close.
70+
* @returns boolean indicating whether the attempt was successful.
71+
*/
72+
async #tryToClose(interfaceId: string): Promise<boolean> {
73+
try {
74+
await this.#snap.request({
75+
method: 'snap_resolveInterface',
76+
params: { id: interfaceId, value: {} },
77+
});
78+
return true;
79+
} catch {
80+
return false;
81+
}
82+
}
83+
84+
/**
85+
* Checks whether the specified interface exists.
86+
* @param interfaceId - The ID of the interface to check.
87+
* @returns boolean indicating whether the interface exists.
88+
*/
89+
async #doesInterfaceExist(interfaceId: string): Promise<boolean> {
90+
try {
91+
await this.#snap.request({
92+
method: 'snap_getInterfaceContext',
93+
params: { id: interfaceId },
94+
});
95+
return true;
96+
} catch {
97+
return false;
98+
}
99+
}
100+
67101
/**
68102
* Programmatically close the dialog.
69103
* Safe to call multiple times.
104+
* Best effort: attempts up to three close requests; always clears local state and resolves.
105+
* Callers in confirmation.tsx do not catch errors, so this method does not throw.
70106
*/
71107
async close(): Promise<void> {
72-
if (this.#interfaceId) {
73-
try {
74-
await this.#snap.request({
75-
method: 'snap_resolveInterface',
76-
params: { id: this.#interfaceId, value: {} },
77-
});
78-
} catch {
79-
// Silently ignore - dialog may already be closed
80-
}
108+
const MAX_ATTEMPTS = 3;
109+
110+
const cleanup = (): void => {
81111
this.#interfaceId = undefined;
82112
this.#isDialogShown = false;
113+
};
114+
115+
if (!this.#interfaceId) {
116+
return;
83117
}
118+
119+
const id = this.#interfaceId;
120+
121+
for (let attempt = 0; attempt < MAX_ATTEMPTS; attempt++) {
122+
if (await this.#tryToClose(id)) {
123+
cleanup();
124+
return;
125+
}
126+
127+
if (!(await this.#doesInterfaceExist(id))) {
128+
cleanup();
129+
return;
130+
}
131+
}
132+
133+
cleanup();
84134
}
85135

86136
/**

packages/gator-permissions-snap/src/index.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,6 @@ const snapsMetricsService = new SnapsMetricsService(snap);
147147

148148
const profileSyncManager = createProfileSyncManager({
149149
isFeatureEnabled: isStorePermissionsFeatureEnabled,
150-
auth,
151150
userStorage: new UserStorage(
152151
{
153152
auth,

packages/gator-permissions-snap/test/core/dialogInterface.test.tsx

Lines changed: 84 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ describe('DialogInterface', () => {
210210
describe('close', () => {
211211
const mockInterfaceId = 'test-interface-123';
212212

213-
it('should resolve the interface', async () => {
213+
it('should resolve the interface and clear state on first attempt', async () => {
214214
mockSnapsProvider.request.mockImplementation(async (params: any) => {
215215
if (params.method === 'snap_createInterface') {
216216
return mockInterfaceId;
@@ -234,13 +234,14 @@ describe('DialogInterface', () => {
234234
value: {},
235235
},
236236
});
237+
expect(dialogInterface.interfaceId).toBeUndefined();
237238
});
238239

239240
it('should be safe to call when no interface exists', async () => {
240-
await expect(dialogInterface.close()).resolves.not.toThrow();
241+
await expect(dialogInterface.close()).resolves.toBeUndefined();
241242
});
242243

243-
it('should silently ignore errors when closing', async () => {
244+
it('should not throw when close fails and interface is already gone', async () => {
244245
mockSnapsProvider.request.mockImplementation(async (params: any) => {
245246
if (params.method === 'snap_createInterface') {
246247
return mockInterfaceId;
@@ -251,11 +252,90 @@ describe('DialogInterface', () => {
251252
if (params.method === 'snap_resolveInterface') {
252253
throw new Error('Already resolved');
253254
}
255+
if (params.method === 'snap_getInterfaceContext') {
256+
throw new Error('Interface not found');
257+
}
258+
return null;
259+
});
260+
261+
await dialogInterface.show(<Text>Test</Text>);
262+
await expect(dialogInterface.close()).resolves.toBeUndefined();
263+
expect(dialogInterface.interfaceId).toBeUndefined();
264+
});
265+
266+
it('should retry and clear state when first close fails but second succeeds', async () => {
267+
let resolveCallCount = 0;
268+
mockSnapsProvider.request.mockImplementation(async (params: any) => {
269+
if (params.method === 'snap_createInterface') {
270+
return mockInterfaceId;
271+
}
272+
if (params.method === 'snap_dialog') {
273+
return new Promise(() => {});
274+
}
275+
if (params.method === 'snap_resolveInterface') {
276+
resolveCallCount += 1;
277+
if (resolveCallCount === 1) {
278+
throw new Error('Temporary failure');
279+
}
280+
return null;
281+
}
282+
if (params.method === 'snap_getInterfaceContext') {
283+
return {};
284+
}
285+
return null;
286+
});
287+
288+
await dialogInterface.show(<Text>Test</Text>);
289+
await dialogInterface.close();
290+
291+
expect(resolveCallCount).toBe(2);
292+
expect(dialogInterface.interfaceId).toBeUndefined();
293+
});
294+
295+
it('should not throw when all close attempts fail', async () => {
296+
mockSnapsProvider.request.mockImplementation(async (params: any) => {
297+
if (params.method === 'snap_createInterface') {
298+
return mockInterfaceId;
299+
}
300+
if (params.method === 'snap_dialog') {
301+
return new Promise(() => {});
302+
}
303+
if (params.method === 'snap_resolveInterface') {
304+
throw new Error('Close failed');
305+
}
306+
if (params.method === 'snap_getInterfaceContext') {
307+
return {};
308+
}
309+
return null;
310+
});
311+
312+
await dialogInterface.show(<Text>Test</Text>);
313+
await expect(dialogInterface.close()).resolves.toBeUndefined();
314+
});
315+
316+
it('should clear state after exhausting retries when all close attempts fail', async () => {
317+
mockSnapsProvider.request.mockImplementation(async (params: any) => {
318+
if (params.method === 'snap_createInterface') {
319+
return mockInterfaceId;
320+
}
321+
if (params.method === 'snap_dialog') {
322+
return new Promise(() => {});
323+
}
324+
if (params.method === 'snap_resolveInterface') {
325+
throw new Error('Close failed');
326+
}
327+
if (params.method === 'snap_getInterfaceContext') {
328+
return {};
329+
}
254330
return null;
255331
});
256332

257333
await dialogInterface.show(<Text>Test</Text>);
258-
await expect(dialogInterface.close()).resolves.not.toThrow();
334+
expect(dialogInterface.interfaceId).toBe(mockInterfaceId);
335+
336+
await dialogInterface.close();
337+
338+
expect(dialogInterface.interfaceId).toBeUndefined();
259339
});
260340
});
261341

0 commit comments

Comments
 (0)