Skip to content

Commit 6ef1fc7

Browse files
authored
feat(SDKConnectV2): add toasts for non-success states (#22610)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This PR adds to the current implementation of MetaMask Connect toasts to ensure that error messages are displayed in the Wallet when a user rejects a request or when other errors occur during the request lifecycle. <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/WAPI-825 ## **Manual testing steps** ```gherkin Feature: MM Connect toasts for non-success states Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** https://github.com/user-attachments/assets/e977ff69-aedd-4136-99a9-9e9e90f5e663 <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Show an error toast when bridge responses contain JSON-RPC errors and use the connection id for error notifications; add tests for success/error paths. > > - **SDKConnectV2**: > - **Connection (`services/connection.ts`)**: On bridge `response`, detect JSON-RPC errors (`error` in payload) and call `hostApp.showConnectionError(this.info)`; otherwise call `showReturnToApp(this.info)`. Always forward payload to client. > - **Host App Adapter (`adapters/host-application-adapter.ts`)**: Change `showConnectionError` signature to `conninfo?` and use `conninfo.id` when provided (fallback to timestamp). > - **Types (`types/host-application-adapter.ts`)**: Update interface for `showConnectionError(conninfo?)` and clarify return-to-app success doc. > - **Tests**: > - Add/update cases validating error vs success toasts and that error notifications use the connection id; ensure responses without `id` don’t trigger return-to-app. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 803a6b5. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 66388aa commit 6ef1fc7

File tree

5 files changed

+99
-5
lines changed

5 files changed

+99
-5
lines changed

app/core/SDKConnectV2/adapters/host-application-adapter.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,22 @@ describe('HostApplicationAdapter', () => {
118118
});
119119
expect(store.dispatch).toHaveBeenCalledTimes(1);
120120
});
121+
122+
it('dispatches an error notification when request is rejected or fails', () => {
123+
adapter.showConnectionError(
124+
createMockConnectionInfo('session-123', 'Test DApp'),
125+
);
126+
127+
expect(showSimpleNotification).toHaveBeenCalledTimes(1);
128+
expect(showSimpleNotification).toHaveBeenCalledWith({
129+
id: 'session-123',
130+
autodismiss: 5000,
131+
title: 'sdk_connect_v2.show_error.title',
132+
description: 'sdk_connect_v2.show_error.description',
133+
status: 'error',
134+
});
135+
expect(store.dispatch).toHaveBeenCalledTimes(1);
136+
});
121137
});
122138

123139
describe('showReturnToApp', () => {

app/core/SDKConnectV2/adapters/host-application-adapter.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,10 @@ export class HostApplicationAdapter implements IHostApplicationAdapter {
3333
store.dispatch(hideNotificationById(conninfo.id));
3434
}
3535

36-
showConnectionError(): void {
36+
showConnectionError(conninfo?: ConnectionInfo): void {
3737
store.dispatch(
3838
showSimpleNotification({
39-
id: Date.now().toString(),
39+
id: conninfo?.id || Date.now().toString(),
4040
autodismiss: 5000,
4141
title: strings('sdk_connect_v2.show_error.title'),
4242
description: strings('sdk_connect_v2.show_error.description'),

app/core/SDKConnectV2/services/connection.test.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,5 +331,74 @@ describe('Connection', () => {
331331
responsePayload,
332332
);
333333
});
334+
335+
it('shows error toast when bridge response includes an error', async () => {
336+
await Connection.create(
337+
mockConnectionInfo,
338+
mockKeyManager,
339+
RELAY_URL,
340+
mockHostApp,
341+
);
342+
343+
const errorResponsePayload = {
344+
data: {
345+
id: 1,
346+
jsonrpc: '2.0',
347+
error: {
348+
code: -32000,
349+
message: 'User rejected the request',
350+
},
351+
},
352+
};
353+
354+
// Simulate the RPCBridgeAdapter emitting an error response
355+
onBridgeResponseCallback(errorResponsePayload);
356+
357+
// Should show error toast, not success toast
358+
expect(mockHostApp.showConnectionError).toHaveBeenCalledTimes(1);
359+
expect(mockHostApp.showConnectionError).toHaveBeenCalledWith(
360+
mockConnectionInfo,
361+
);
362+
expect(mockHostApp.showReturnToApp).not.toHaveBeenCalled();
363+
364+
// And still send the error response to the client
365+
expect(mockWalletClientInstance.sendResponse).toHaveBeenCalledTimes(1);
366+
expect(mockWalletClientInstance.sendResponse).toHaveBeenCalledWith(
367+
errorResponsePayload,
368+
);
369+
});
370+
371+
it('shows success toast for successful response with result', async () => {
372+
await Connection.create(
373+
mockConnectionInfo,
374+
mockKeyManager,
375+
RELAY_URL,
376+
mockHostApp,
377+
);
378+
379+
const successResponsePayload = {
380+
data: {
381+
id: 1,
382+
jsonrpc: '2.0',
383+
result: ['0x123'],
384+
},
385+
};
386+
387+
// Simulate the RPCBridgeAdapter emitting a success response
388+
onBridgeResponseCallback(successResponsePayload);
389+
390+
// Should show success toast, not error toast
391+
expect(mockHostApp.showReturnToApp).toHaveBeenCalledTimes(1);
392+
expect(mockHostApp.showReturnToApp).toHaveBeenCalledWith(
393+
mockConnectionInfo,
394+
);
395+
expect(mockHostApp.showConnectionError).not.toHaveBeenCalled();
396+
397+
// And still send the response to the client
398+
expect(mockWalletClientInstance.sendResponse).toHaveBeenCalledTimes(1);
399+
expect(mockWalletClientInstance.sendResponse).toHaveBeenCalledWith(
400+
successResponsePayload,
401+
);
402+
});
334403
});
335404
});

app/core/SDKConnectV2/services/connection.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,16 @@ export class Connection {
4646

4747
// If the payload includes an id, its a JSON-RPC response, otherwise its a notification
4848
if ('data' in payload && 'id' in payload.data) {
49-
this.hostApp.showReturnToApp(this.info);
49+
const responseData = payload.data;
50+
// Check if the response is an error (JSON-RPC error responses have an 'error' property)
51+
const isError =
52+
'error' in responseData && responseData.error !== undefined;
53+
54+
if (isError) {
55+
this.hostApp.showConnectionError(this.info);
56+
} else {
57+
this.hostApp.showReturnToApp(this.info);
58+
}
5059
}
5160

5261
this.client.sendResponse(payload);

app/core/SDKConnectV2/types/host-application-adapter.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,10 @@ export interface IHostApplicationAdapter {
2323
/**
2424
* Displays a global, non-interactive error modal.
2525
*/
26-
showConnectionError(): void;
26+
showConnectionError(conninfo?: ConnectionInfo): void;
2727

2828
/**
29-
* Displays a "Return to App" toast notification.
29+
* Displays a "Return to App" toast notification for successful requests.
3030
*/
3131
showReturnToApp(conninfo: ConnectionInfo): void;
3232

0 commit comments

Comments
 (0)