Skip to content

Commit aad9491

Browse files
fix: Cursor review
1 parent 00dc08a commit aad9491

2 files changed

Lines changed: 67 additions & 8 deletions

File tree

app/core/DeeplinkManager/handlers/legacy/__tests__/handleUniversalLink.test.ts

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import handleDeepLinkModalDisplay from '../handleDeepLinkModalDisplay';
1515
import handleBrowserUrl from '../handleBrowserUrl';
1616
import { DeepLinkModalLinkType } from '../../../../../components/UI/DeepLinkModal';
1717
import handleMetaMaskDeeplink from '../handleMetaMaskDeeplink';
18+
import Logger from '../../../../../util/Logger';
19+
import type { DeepLinkAnalyticsContext } from '../../../types/deepLinkAnalytics.types';
1820
import { SHIELD_WEBSITE_URL } from '../../../../../constants/shield';
1921
// eslint-disable-next-line import-x/no-namespace
2022
import * as signatureUtils from '../../../utils/verifySignature';
@@ -130,6 +132,11 @@ describe('handleUniversalLink', () => {
130132
beforeEach(() => {
131133
jest.clearAllMocks();
132134

135+
// `handleMetaMaskDeeplink` is async and its returned promise is
136+
// `.catch`ed by the call site. The auto-mock returns `undefined`, so
137+
// restore the async contract for all tests by default.
138+
mockHandleMetaMaskDeeplink.mockResolvedValue(undefined);
139+
133140
mockSDKConnectGetInstance.mockImplementation(() => ({
134141
getConnections: jest.fn(),
135142
connectToChannel: jest.fn(),
@@ -178,6 +185,46 @@ describe('handleUniversalLink', () => {
178185
});
179186
},
180187
);
188+
189+
// `handleMetaMaskDeeplink` is async and deliberately not awaited here.
190+
// Make sure rejections are observed (logged) rather than surfacing as
191+
// unhandled promise rejections — this covers the synchronous security
192+
// throw for `INTERNAL_ORIGINS` and any rejection from `SDKConnect.init`.
193+
it('swallows and logs rejections from handleMetaMaskDeeplink', async () => {
194+
const loggerErrorSpy = jest
195+
.spyOn(Logger, 'error')
196+
.mockImplementation(() => undefined);
197+
const rejectionError = new Error(
198+
'External transactions cannot use internal origins',
199+
);
200+
mockHandleMetaMaskDeeplink.mockRejectedValueOnce(rejectionError);
201+
202+
const testUrl = `https://link.metamask.io/${ACTIONS.CONNECT}`;
203+
const { urlObj: testUrlObj } = extractURLParams(
204+
`metamask://${ACTIONS.CONNECT}`,
205+
);
206+
207+
await expect(
208+
handleUniversalLink({
209+
instance,
210+
handled,
211+
urlObj: testUrlObj,
212+
browserCallBack: mockBrowserCallBack,
213+
url: testUrl,
214+
source: 'origin',
215+
}),
216+
).resolves.not.toThrow();
217+
218+
// Flush the promise attached via `.catch` at the call site.
219+
await Promise.resolve();
220+
221+
expect(loggerErrorSpy).toHaveBeenCalledWith(
222+
rejectionError,
223+
'DeepLinkManager: handleMetaMaskDeeplink failed',
224+
);
225+
226+
loggerErrorSpy.mockRestore();
227+
});
181228
});
182229

183230
describe('ACTIONS.BUY_CRYPTO', () => {
@@ -1994,7 +2041,7 @@ describe('handleUniversalLink', () => {
19942041

19952042
let mockAnalytics: MockMetricsInstance;
19962043
let mockCreateEventBuilder: jest.MockedFunction<
1997-
() => Promise<MockEventBuilder>
2044+
(context: DeepLinkAnalyticsContext) => Promise<MockEventBuilder>
19982045
>;
19992046
const { analytics } = jest.requireMock(
20002047
'../../../../../util/analytics/analytics',
@@ -2023,13 +2070,14 @@ describe('handleUniversalLink', () => {
20232070
};
20242071
analytics.trackEvent = mockAnalytics.trackEvent;
20252072

2026-
mockCreateEventBuilder = jest.fn(() =>
2027-
Promise.resolve({
2028-
addProperties: jest.fn().mockReturnThis(),
2029-
addSensitiveProperties: jest.fn().mockReturnThis(),
2030-
build: jest.fn().mockReturnValue({ eventName: 'DEEP_LINK_USED' }),
2031-
}),
2032-
);
2073+
mockCreateEventBuilder = jest.fn(
2074+
(_context: DeepLinkAnalyticsContext) =>
2075+
Promise.resolve({
2076+
addProperties: jest.fn().mockReturnThis(),
2077+
addSensitiveProperties: jest.fn().mockReturnThis(),
2078+
build: jest.fn().mockReturnValue({ eventName: 'DEEP_LINK_USED' }),
2079+
}),
2080+
) as typeof mockCreateEventBuilder;
20332081
createDeepLinkUsedEventBuilder.mockImplementation(mockCreateEventBuilder);
20342082
});
20352083

app/core/DeeplinkManager/handlers/legacy/handleUniversalLink.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,12 +206,23 @@ async function handleUniversalLink({
206206
);
207207
const { urlObj: mappedUrlObj, params } = extractURLParams(mappedUrl);
208208
const wcURL = params?.uri || mappedUrlObj.href;
209+
// `handleMetaMaskDeeplink` is async (it awaits `SDKConnect.init` so the
210+
// connect/mmsdk branches can't race init). We deliberately don't await
211+
// it here — the call is fire-and-forget from the deeplink pipeline's
212+
// perspective — but we must still surface rejections (including the
213+
// synchronous `INTERNAL_ORIGINS` security throw) so they don't become
214+
// silent unhandled promise rejections.
209215
handleMetaMaskDeeplink({
210216
handled,
211217
wcURL,
212218
origin: source,
213219
params,
214220
url: mappedUrl,
221+
}).catch((err) => {
222+
Logger.error(
223+
err as Error,
224+
'DeepLinkManager: handleMetaMaskDeeplink failed',
225+
);
215226
});
216227
return;
217228
}

0 commit comments

Comments
 (0)