Skip to content

Commit

Permalink
fix: Return error as sent from the UI instead of throwing a new one (#…
Browse files Browse the repository at this point in the history
…4610)

## Explanation

When signatures are rejected the UI sends some extra information in the
error data property. This information is stripped off when the error is
re-thrown in the controller. This change aims as throwing the error back
to the UI as received.

Removes the @metamask/rpc-errors package from the Signature controller
as this is not used anymore.
## References

<!--
Are there any issues that this pull request is tied to? Are there other
links that reviewers should consult to understand these changes better?

For example:

* Fixes #12345
* Related to
[#2703](MetaMask/MetaMask-planning#2703)
-->

## Changelog

None


## Checklist

- [ ] I've updated the test suite for new or updated code as appropriate
- [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [ ] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
  • Loading branch information
pnarayanaswamy authored Aug 15, 2024
1 parent a6177fb commit d1202fd
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 12 deletions.
1 change: 0 additions & 1 deletion packages/signature-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
"@metamask/base-controller": "^6.0.2",
"@metamask/controller-utils": "^11.0.2",
"@metamask/message-manager": "^10.0.2",
"@metamask/rpc-errors": "^6.3.1",
"@metamask/utils": "^9.1.0",
"lodash": "^4.17.21"
},
Expand Down
21 changes: 14 additions & 7 deletions packages/signature-controller/src/SignatureController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
PersonalMessageManager,
TypedMessageManager,
} from '@metamask/message-manager';
import { EthereumProviderError } from '@metamask/rpc-errors';

import type {
SignatureControllerMessenger,
Expand Down Expand Up @@ -368,18 +367,23 @@ describe('SignatureController', () => {
it('throws if approval rejected', async () => {
messengerMock.call
.mockResolvedValueOnce({}) // LoggerController:add
.mockRejectedValueOnce({}); // ApprovalController:addRequest
.mockRejectedValueOnce({
message: 'User rejected the request.',
data: {
source: 'MetaMask',
},
}); // ApprovalController:addRequest
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const error: any = await getError(
const error: any = await getError<Error>(
async () =>
await signatureController.newUnsignedPersonalMessage(
messageParamsMock,
requestMock,
),
);
expect(error instanceof EthereumProviderError).toBe(true);
expect(error.message).toBe('User rejected the request.');
expect(error.data).toStrictEqual({ source: 'MetaMask' });
});

it('throws if cannot get signature', async () => {
Expand Down Expand Up @@ -521,10 +525,13 @@ describe('SignatureController', () => {
it('throws if approval rejected', async () => {
messengerMock.call
.mockResolvedValueOnce({}) // LoggerController:add
.mockRejectedValueOnce({}); // ApprovalController:addRequest
.mockRejectedValueOnce({
message: 'User rejected the request.',
data: { source: 'Metamask' },
}); // ApprovalController:addRequest
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const error: any = await getError(
const error: any = await getError<Error>(
async () =>
await signatureController.newUnsignedTypedMessage(
messageParamsMock,
Expand All @@ -533,8 +540,8 @@ describe('SignatureController', () => {
{ parseJsonData: true },
),
);
expect(error instanceof EthereumProviderError).toBe(true);
expect(error.message).toBe('User rejected the request.');
expect(error.data).toStrictEqual({ source: 'Metamask' });
});

it('throws if cannot get signature', async () => {
Expand Down
5 changes: 2 additions & 3 deletions packages/signature-controller/src/SignatureController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import {
PersonalMessageManager,
TypedMessageManager,
} from '@metamask/message-manager';
import { providerErrors } from '@metamask/rpc-errors';
import type { Hex, Json } from '@metamask/utils';
import EventEmitter from 'events';
import { cloneDeep } from 'lodash';
Expand Down Expand Up @@ -432,7 +431,7 @@ export class SignatureController extends BaseController<
);

resultCallbacks = acceptResult.resultCallbacks;
} catch {
} catch (error) {
// User rejected the signature request
this.#addLog(
signTypeForLogger,
Expand All @@ -441,7 +440,7 @@ export class SignatureController extends BaseController<
);

this.#cancelAbstractMessage(messageManager, messageId);
throw providerErrors.userRejectedRequest('User rejected the request.');
throw error;
}

// TODO: Either fix this lint violation or explain why it's necessary to ignore.
Expand Down
1 change: 0 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3711,7 +3711,6 @@ __metadata:
"@metamask/keyring-controller": "npm:^17.1.2"
"@metamask/logging-controller": "npm:^5.0.0"
"@metamask/message-manager": "npm:^10.0.2"
"@metamask/rpc-errors": "npm:^6.3.1"
"@metamask/utils": "npm:^9.1.0"
"@types/jest": "npm:^27.4.1"
deepmerge: "npm:^4.2.2"
Expand Down

0 comments on commit d1202fd

Please sign in to comment.