Skip to content

Commit

Permalink
refactor: revamp snap index.ts error handle (#343)
Browse files Browse the repository at this point in the history
* chore: update logger to deine from build

* chore: update default log level config

* chore: revamp index error handle

* chore: update error handle

---------

Co-authored-by: khanti42 <[email protected]>
  • Loading branch information
stanleyyconsensys and khanti42 authored Sep 4, 2024
1 parent 3acbacf commit f830c34
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 288 deletions.
30 changes: 23 additions & 7 deletions packages/starknet-snap/src/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { MethodNotFoundError, SnapError } from '@metamask/snaps-sdk';
import { constants } from 'starknet';

import { onRpcRequest, onHomePage } from '.';
Expand Down Expand Up @@ -50,15 +51,30 @@ describe('onRpcRequest', () => {
expect(createAccountSpy).toHaveBeenCalledTimes(1);
});

it('throws `Unable to execute the rpc request` error if an error has thrown', async () => {
const { createAccountSpy, keyPairSpy } = createMockSpy();
it('throws `MethodNotFoundError` if the request method not found', async () => {
await expect(
onRpcRequest({
...createMockRequest(),
request: {
...createMockRequest().request,
method: 'method_not_found',
},
}),
).rejects.toThrow(MethodNotFoundError);
});

createAccountSpy.mockRejectedValue(new Error('Custom Error'));
keyPairSpy.mockReturnThis();
it('throws `SnapError` if the error is an instance of SnapError', async () => {
const { createAccountSpy } = createMockSpy();
createAccountSpy.mockRejectedValue(new SnapError('error'));

await expect(onRpcRequest(createMockRequest())).rejects.toThrow(
'Unable to execute the rpc request',
);
await expect(onRpcRequest(createMockRequest())).rejects.toThrow(SnapError);
});

it('throws `SnapError` if the error is not an instance of SnapError', async () => {
const { createAccountSpy } = createMockSpy();
createAccountSpy.mockRejectedValue(new Error('error'));

await expect(onRpcRequest(createMockRequest())).rejects.toThrow(SnapError);
});
});

Expand Down
23 changes: 12 additions & 11 deletions packages/starknet-snap/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
text,
copyable,
SnapError,
MethodNotFoundError,
} from '@metamask/snaps-sdk';
import { ethers } from 'ethers';

Expand Down Expand Up @@ -62,6 +63,7 @@ import type {
} from './types/snapApi';
import type { SnapState } from './types/snapState';
import { upgradeAccContract } from './upgradeAccContract';
import { isSnapRpcError } from './utils';
import {
CAIRO_VERSION_LEGACY,
ETHER_MAINNET,
Expand All @@ -73,7 +75,7 @@ import {
} from './utils/constants';
import { getAddressKeyDeriver } from './utils/keyPair';
import { acquireLock } from './utils/lock';
import { logger, LogLevel } from './utils/logger';
import { logger } from './utils/logger';
import { toJson } from './utils/serializer';
import {
dappUrl,
Expand Down Expand Up @@ -293,19 +295,18 @@ export const onRpcRequest: OnRpcRequestHandler = async ({ request }) => {
return await getStarkName(apiParams);

default:
throw new Error('Method not found.');
throw new MethodNotFoundError() as unknown as Error;
}
} catch (error) {
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
logger.error(`Error: ${error}`);
// We don't want to expose the error message to the user when it is a production build
if (logger.logLevel === LogLevel.OFF) {
throw new SnapError(
'Unable to execute the rpc request',
) as unknown as Error;
} else {
throw new SnapError(error.message) as unknown as Error;
let snapError = error;

if (!isSnapRpcError(error)) {
snapError = new SnapError('Unable to execute the rpc request');
}
logger.error(
`onRpcRequest error: ${JSON.stringify(snapError.toJSON(), null, 2)}`,
);
throw snapError;
}
};

Expand Down
20 changes: 0 additions & 20 deletions packages/starknet-snap/src/utils/rpc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,26 +85,6 @@ describe('RpcController', () => {

expect(result).toBe('done test');
});

it('throws `Failed to execute the rpc method` if an error was thrown', async () => {
const rpc = new MockRpc();

jest
.spyOn(MockRpc.prototype, 'handleRequest')
.mockRejectedValue(new Error('error'));

await expect(rpc.execute('test')).rejects.toThrow(
'Failed to execute the rpc method',
);
});

it('throws the actual error if an snap error was thrown', async () => {
const rpc = new MockRpc();

await expect(rpc.execute(1 as unknown as string)).rejects.toThrow(
'Expected a string, but received: 1',
);
});
});

describe('AccountRpcController', () => {
Expand Down
18 changes: 4 additions & 14 deletions packages/starknet-snap/src/utils/rpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,20 +78,10 @@ export abstract class RpcController<
* @returns A promise that resolves to an json.
*/
async execute(params: Request): Promise<Response> {
try {
await this.preExecute(params);
const resp = await this.handleRequest(params);
await this.postExecute(resp);
return resp;
} catch (error) {
logger.error('Failed to execute the rpc method', error);

if (error instanceof SnapError) {
throw error as unknown as Error;
}

throw new Error('Failed to execute the rpc method');
}
await this.preExecute(params);
const resp = await this.handleRequest(params);
await this.postExecute(resp);
return resp;
}
}

Expand Down
236 changes: 0 additions & 236 deletions packages/starknet-snap/test/src/index.test.ts

This file was deleted.

0 comments on commit f830c34

Please sign in to comment.